Re: epg--status-GET-HIDDEN cleanup suggestion

[Ted Zlatanov <tzz@lifelogs.com>]

On Sat, 16 Aug 2008 22:41:42 +0900 Daiki Ueno <ueno@unixuser.org> wrote: 

DU> Ted Zlatanov <tzz@lifelogs.com> writes:
>> The function epg--status-GET-HIDDEN in epg.el had lots of repetition, so
>> I cleaned it up a little with a let*.

DU> Lots of?  It seems that your change binds only a few variables, and
DU> doesn't reduce the lines of code.

You called (epg-context-passphrase-callback context) 6 times.  My change
reduces it to 1 call, bound to a variable from that point on.  Also, I
changed

(funcall
      (if (consp (epg-context-passphrase-callback context))
          (car (epg-context-passphrase-callback context))
        (epg-context-passphrase-callback context))
      context
      epg-key-id
      (if (consp (epg-context-passphrase-callback context))
          (cdr (epg-context-passphrase-callback context))))

to

(let* ((context-callback (epg-context-passphrase-callback context))
       (callback (or (car-safe context-callback) context-callback))
       (file (cdr-safe context-callback)))
...
 (funcall callback context epg-key-id file))

which to me seems much more readable (using car-safe and cdr-safe makes
the logic simpler too).  Lines of code are not the only measure of code
cleanliness; in this specific case I found your logic hard to read while
debugging an unrelated problem so I tried to clean it up.

>> I don't know the protocol for comitting these cleanups, so I offer the
>> patch for review.  It works for me.

DU> I don't like to bind variables which is used only once at run time.

Your choice, it's your code.  I find my version more readable, but
that's certainly a matter of opinion.

Ted