[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Ted Zlatanov
Subject: Re: epg--status-GET-HIDDEN cleanup suggestion
Date: Sat, 16 Aug 2008 11:05:32 -0500
User-agent: Gnus/5.110011 (No Gnus v0.11) Emacs/23.0.60 (darwin)

On Sat, 16 Aug 2008 22:41:42 +0900 Daiki Ueno <address@hidden> wrote: 

DU> Ted Zlatanov <address@hidden> 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

      (if (consp (epg-context-passphrase-callback context))
          (car (epg-context-passphrase-callback context))
        (epg-context-passphrase-callback context))
      (if (consp (epg-context-passphrase-callback context))
          (cdr (epg-context-passphrase-callback context))))


(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.


reply via email to

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