[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: 5x5 Arithmetic solver
From: |
Stefan Monnier |
Subject: |
Re: 5x5 Arithmetic solver |
Date: |
Fri, 20 May 2011 10:45:02 -0300 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) |
> Please find herein attached a contribution to the 5x5 game. This is an
> arithmetic solver based on a matrix inversion in a (Z/2Z)^25 vector
> space.
Thanks. A few comments:
- avoid using a tarball and just attach the diff as-is,
makes it a lot easier to review.
- why 5x5-local-variables?
- explain the changes in the 5x5 function.
- many of your lines have trailing whitespace. I generally don't care
much, about it, but some people do, and it's usually preferable to
avoid them. M-x picture-mode C-c C-c gets rid of them for you (as
a side-effect).
- try to keep the first line of docstrings as a self-sufficient sentence
(because M-x apropos only shows the first line).
- stay within 80 columns.
- your code is not properly indented (e.g. the `grid' argument in
5x5-grid-to-vec).
- Please capitalize your comments and terminate them with a "." or some
other appropriate punctuation.
- 5x5-solve-suggest should have a docstring.
- try C-u checkdoc-current-buffer.
- we need a ChangeLog entry.
- I don't understand the "solve step" message (e.g. it said 23 every
time, even though I followed its suggestions and finished in 12 moves).
Stefan