unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Drew Adams <drew.adams@oracle.com>
To: Glenn Morris <rgm@gnu.org>, Karl Fogel <kfogel@red-bean.com>
Cc: Ivan Kanis <ivan@kanis.fr>,
	Stefan Monnier <monnier@iro.umontreal.ca>,
	Emacs Development List <emacs-devel@gnu.org>
Subject: RE: code review request for saveplace with dired buffer
Date: Fri, 7 Jun 2013 22:09:39 -0700 (PDT)	[thread overview]
Message-ID: <7cec361d-9286-42e1-94eb-379bba6620ff@default> (raw)
In-Reply-To: <bf125801-8bea-4524-89c6-66b64637a3f1@default>

I mentioned:

> A Common Lisp convention is to use `when' and `unless' when the
> return value is not important.  It communicates to a (human) reader
> that the body is for side effects only.
> 
> I, for one, use this same convention with Emacs Lisp.  I consider
> code that depends on the return value of `when' or `unless' to be
> bad style (misleading).  Some will disagree, no doubt.

I was pretty sure that this style guidance was even called out in the
Common Lisp spec/manual, but I hadn't bothered to look it up again
(and it's been decades since I read it).

And so it is.  Here is what "Common Lisp The Language 2nd Ed." says:

    "As a matter of style, `when' is normally used to conditionally
   produce some side effects, and the value of the `when' form is
   normally not used. If the value is relevant, then it may be
   stylistically more appropriate to use `and' or `if'."

IMO, it's too bad that Emacs Dev does not appreciate & adopt such
a convention, as it helps users by informally communicating
programmer intent.

Yes, of course any such informal communication can mislead if
applied incorrectly, just as can an incorrect comment.  Still,
it makes sense.


To add to this, I'll mention that if the return value is important
then I also tend to use `and' instead of `if' with no else clause.
E.g.:

(use-value (and some-conditon  return-value))

vs
(use-value (if some-condition return-value))

I don't claim this second style guideline is a Common Lisp convention.

But as a consequence of these two rules, if you see an `if' in my
code then you can expect one or more else clauses.  If you find an
`if' with no else clause then it probably does not reflect what I
intended - what I thought I coded (likely a bug somewhere).

A third rule I use: I generally avoid using `if' with a literal `nil'
as result.  The condition can be negated to get rid of the `nil',
which would, by the second rule, be replaced by `and'.  E.g.:

(use-value (and condition  return-val))

vs
(use-value (if condition return-val))           ; 2nd rule
or
(use-value (if condition return-val nil)        ; 3rd rule
or
(use-value (if (not condition) nil return-val)) ; 3rd rule


An exception to the 3rd rule can be when `nil' is used as the empty
list.  But in that case I (try to remember to) write it as `()',
not `nil'.  E.g.: (use-list (if condition () other-list))

I don't always do that, though - sometimes even for a list I use
(and (not condition)  other-list).  Depends how much attention I
want to draw to the empty-list case, I guess.

I sometimes use another exception to the 3rd rule, to highlight
`nil' as a return value, especially if there are many side-effect
else clauses or some of them are complex.  E.g.:

(defun foo ()
  (if condition
      nil        ; Return nil
    (do-aaaa)
    (do-bbbb)
    (do-cccc)
    ...
    (do-zzzz)
    result))

That can sometimes be clearer than, say:

(defun foo ()
  (and condition
       (progn (do-aaaa)
              (do-bbbb)
              (do-cccc)
              ...
              (do-zzzz)
              result)))

(I tend to avoid `progn' too.)



  parent reply	other threads:[~2013-06-08  5:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-07  6:43 code review request for saveplace with dired buffer Ivan Kanis
2013-06-07 15:59 ` Karl Fogel
2013-06-07 17:25   ` Glenn Morris
2013-06-07 20:08     ` Drew Adams
2013-06-07 22:20       ` Karl Fogel
2013-06-08  5:09       ` Drew Adams [this message]
2013-06-07 19:12 ` Stefan Monnier
2013-06-09 15:42   ` Ivan Kanis
2013-06-09 16:20     ` Karl Fogel
2013-06-10  7:42       ` Ivan Kanis
2013-06-10 14:00         ` Karl Fogel
2013-06-10 16:35     ` Glenn Morris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7cec361d-9286-42e1-94eb-379bba6620ff@default \
    --to=drew.adams@oracle.com \
    --cc=emacs-devel@gnu.org \
    --cc=ivan@kanis.fr \
    --cc=kfogel@red-bean.com \
    --cc=monnier@iro.umontreal.ca \
    --cc=rgm@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).