emacs-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]