Elisp programming style

Elisp programming style

[Thorsten]

Hi List, 
doing some Elisp programming (in an orgmode context) recently, the
following question with regards to the 'accepted programming style' for
Elisp concerned me:

How independent and self-sustained should helper functions be? 

I found some redundancy in elisp code, e.g. several (main and helper)
functions that do exactly the same thing to extract the same specific
args out of an argument list. My first reaction was, to factor out this
extraction into the main function, and then call the helper functions
from inside a (let ...) environment where the extracted args are stored
in a local variable.

But then I recognised, that the helper functions cannot be called
independently anymore, but only make sense when called from this one
main function with its local bindings.

Is there a kind of convention in a case like this? Like: "Make every function,
even a helper function, independent, and don't care about redundancy"?
Just being curious

cheers
-- 
Thorsten

Re: Elisp programming style

[Štěpán Němec]

On Thu, 27 Oct 2011 20:03:22 +0200
Thorsten wrote:

> Hi List, 
> doing some Elisp programming (in an orgmode context) recently, the
> following question with regards to the 'accepted programming style' for
> Elisp concerned me:
>
> How independent and self-sustained should helper functions be? 
>
> I found some redundancy in elisp code, e.g. several (main and helper)
> functions that do exactly the same thing to extract the same specific
> args out of an argument list. My first reaction was, to factor out this
> extraction into the main function, and then call the helper functions
> from inside a (let ...) environment where the extracted args are stored
> in a local variable.
>
> But then I recognised, that the helper functions cannot be called
> independently anymore, but only make sense when called from this one
> main function with its local bindings.
>
> Is there a kind of convention in a case like this? Like: "Make every function,
> even a helper function, independent, and don't care about redundancy"?
> Just being curious

Too bad you didn't give any concrete examples.

In general, if the helper function is only used in a single place and
isn't likely to be more generally useful, there might be no reason for
it to be a standalone function at all (other than modular programming,
which is always a good idea IMO).

OTOH, if the helper function _is_ standalone and used in multiple
places, then there is no "redundancy", so I'm really not sure what you
mean.

If I understand you correctly, you find multiple pairs of main and
helper functions, where the helper does very similar things. In that
case the right thing would seem to be to abstract all the helpers into a
single helper and use that in all the main functions.

[To clarify, this all is just an opinion of an interested Emacs and Org
user who often pulls his hair when trying to make use of/fix bugs in
existing code and bumping into lacks in modular design. The maintainers
will perhaps provide more authoritative guidelines.]

-- 
Štěpán

Re: Elisp programming style

[Thorsten]

Štěpán Němec <stepnem@gmail.com> writes:

> On Thu, 27 Oct 2011 20:03:22 +0200
> Thorsten wrote:
>
>> Hi List, doing some Elisp programming (in an orgmode context)
>> recently, the following question with regards to the 'accepted
>> programming style' for Elisp concerned me:
>>
>> How independent and self-sustained should helper functions be?
>>
>> I found some redundancy in elisp code, e.g. several (main and helper)
>> functions that do exactly the same thing to extract the same specific
>> args out of an argument list. My first reaction was, to factor out
>> this extraction into the main function, and then call the helper
>> functions from inside a (let ...) environment where the extracted
>> args are stored in a local variable.
>>
>> But then I recognised, that the helper functions cannot be called
>> independently anymore, but only make sense when called from this one
>> main function with its local bindings.
>>
>> Is there a kind of convention in a case like this? Like: "Make every
>> function, even a helper function, independent, and don't care about
>> redundancy"? Just being curious
>
> Too bad you didn't give any concrete examples.

The problem can be described easily: 

problem-specific helper-funcions (some redundancy avoided)
,-----------------------------------------------------------
| (defun main-function (args)                               
| (let ((var (assoc :key1 args)))  ; extracting var once
| ...                                                       
| (helper-function1 ...) ; inside let using var             
| (helper-function2 ...) ; inside let using var             
| ))                                                        
|                                                           
| (defun helper-function1 ()                                
| ...                                                       
| )                                                         
|                                                           
| (defun helper-function2 ()                                
| ...                                                       
| )                                                         
`-----------------------------------------------------------

vs

standalone helper-functions (but redundancy)
,-------------------------------------------------------------
| (defun main-function (args)                                 
| (let ((value (assoc :key1 args))   ; extracting var 1st time
| ...                                                         
| )                                                           
| (helper-function1 ...) ; outside let                        
| (helper-function2 ...) ; outside let                        
| )                                                           
|                                                             
| (defun helper-function1 (args)                              
| (let ((var (assoc :key1 args)))  ; extracting var 2nd time  
| ...                                                         
| ))                                                          
|                                                             
| (defun helper-function2 (args)                              
| (let ((var (assoc :key1 args)))  ; extracting var 3rd time  
| ...                                                         
| ))                                                          
`-------------------------------------------------------------


> In general, if the helper function is only used in a single place and
> isn't likely to be more generally useful, there might be no reason for
> it to be a standalone function at all (other than modular programming,
> which is always a good idea IMO).
>
> OTOH, if the helper function _is_ standalone and used in multiple
> places, then there is no "redundancy", so I'm really not sure what you
> mean.

Is there a policy? Lets say, an org-library author thinks his helper
functions will be of no use for nobody else and designs them
non-standalone. A few years later somebody wants to write a similar
library and trys to reuse some of the old helper functions, to no avail. 

Would that be considered bad style from the original author, or is that
up to personal choice and not considered a problem?

cheers
-- 
Thorsten

Re: Elisp programming style

[Nick Dokos]

Thorsten <quintfall@googlemail.com> wrote:

> Štěpán Němec <stepnem@gmail.com> writes:
> 
> > On Thu, 27 Oct 2011 20:03:22 +0200
> > Thorsten wrote:
> >
> >> Hi List, doing some Elisp programming (in an orgmode context)
> >> recently, the following question with regards to the 'accepted
> >> programming style' for Elisp concerned me:
> >>
> >> How independent and self-sustained should helper functions be?
> >>
> >> I found some redundancy in elisp code, e.g. several (main and helper)
> >> functions that do exactly the same thing to extract the same specific
> >> args out of an argument list. My first reaction was, to factor out
> >> this extraction into the main function, and then call the helper
> >> functions from inside a (let ...) environment where the extracted
> >> args are stored in a local variable.
> >>
> >> But then I recognised, that the helper functions cannot be called
> >> independently anymore, but only make sense when called from this one
> >> main function with its local bindings.
> >>
> >> Is there a kind of convention in a case like this? Like: "Make every
> >> function, even a helper function, independent, and don't care about
> >> redundancy"? Just being curious
> >
> > Too bad you didn't give any concrete examples.
> 
> The problem can be described easily: 
> 
> problem-specific helper-funcions (some redundancy avoided)
> ,-----------------------------------------------------------
> | (defun main-function (args)                               
> | (let ((var (assoc :key1 args)))  ; extracting var once
> | ...                                                       
> | (helper-function1 ...) ; inside let using var             
> | (helper-function2 ...) ; inside let using var             
> | ))                                                        
> |                                                           
> | (defun helper-function1 ()                                
> | ...                                                       
> | )                                                         
> |                                                           
> | (defun helper-function2 ()                                
> | ...                                                       
> | )                                                         
> `-----------------------------------------------------------
> 

You probably need let* though.

> vs
> 
> standalone helper-functions (but redundancy)
> ,-------------------------------------------------------------
> | (defun main-function (args)                                 
> | (let ((value (assoc :key1 args))   ; extracting var 1st time
> | ...                                                         
> | )                                                           
> | (helper-function1 ...) ; outside let                        
> | (helper-function2 ...) ; outside let                        
> | )                                                           
> |                                                             
> | (defun helper-function1 (args)                              
> | (let ((var (assoc :key1 args)))  ; extracting var 2nd time  
> | ...                                                         
> | ))                                                          
> |                                                             
> | (defun helper-function2 (args)                              
> | (let ((var (assoc :key1 args)))  ; extracting var 3rd time  
> | ...                                                         
> | ))                                                          
> `-------------------------------------------------------------
> 
> 
> > In general, if the helper function is only used in a single place and
> > isn't likely to be more generally useful, there might be no reason for
> > it to be a standalone function at all (other than modular programming,
> > which is always a good idea IMO).
> >
> > OTOH, if the helper function _is_ standalone and used in multiple
> > places, then there is no "redundancy", so I'm really not sure what you
> > mean.
> 
> Is there a policy? Lets say, an org-library author thinks his helper
> functions will be of no use for nobody else and designs them
> non-standalone. A few years later somebody wants to write a similar
> library and trys to reuse some of the old helper functions, to no avail. 
>

No policy that I know of. It's very much a matter of taste, how complicated
the functions are, whether they perform something generic or something really
specific etc.

> Would that be considered bad style from the original author, or is that
> up to personal choice and not considered a problem?
> 

Not a problem - you can't predict the future. You do the best you can
with your current knowledge. You can always refactor in the future:
that's one reason that having the source code around is important.  When
you refactor, you may be cursing the original author for the "bad"
decicions, but c'est la vie: at least, you have chosen a system where
*you* can do that and not depend on a third party: they might not even
be alive, let alone willing to do what you want.

Nick

Re: Elisp programming style

[Thorsten]

Nick Dokos <nicholas.dokos@hp.com> writes:

> Not a problem - you can't predict the future. You do the best you can
> with your current knowledge. You can always refactor in the future:
> that's one reason that having the source code around is important.  When
> you refactor, you may be cursing the original author for the "bad"
> decicions, but c'est la vie: at least, you have chosen a system where
> *you* can do that and not depend on a third party: they might not even
> be alive, let alone willing to do what you want.

Thats really great about Emacs lisp - everything is so easily
accessible, it seems like no big deal to change something. I still have
to get rid of the reflex to factor out every duplication of code that I
see. Lisp is not Java...

cheers
-- 
Thorsten

Re: Elisp programming style

[Eric Abrahamsen]

[...]


>> Would that be considered bad style from the original author, or is that
>> up to personal choice and not considered a problem?
>> 
>
> Not a problem - you can't predict the future. You do the best you can
> with your current knowledge. You can always refactor in the future:
> that's one reason that having the source code around is important.  When
> you refactor, you may be cursing the original author for the "bad"
> decicions, but c'est la vie: at least, you have chosen a system where
> *you* can do that and not depend on a third party: they might not even
> be alive, let alone willing to do what you want.

I think this is probably the key. As a non-professional programmer who
has gone on safari through the org-mode code (though this obviously
applies to a lot of elisp packages), I think incremental growth is
responsible for a lot of oddities. Refactoring is possible in theory,
but for fast-growing packages it never becomes a priority, and my guess
is by this point there are parts of org where even Carsten would tread
very lightly. It's not a disaster, but it does lead to regression bugs,
which I think we're seeing.

Entropy is contravened, however: witness the push towards an org test
suite, and the production of a generic exporter in org…

Slightly OT, sorry,

Eric

-- 
GNU Emacs 24.0.90.1 (i686-pc-linux-gnu, GTK+ Version 2.24.4)
 of 2011-10-06 on pellet
Org-mode version 7.7 (release_7.7.466.ga5cb)

Re: Elisp programming style

[Tassilo Horn]

Thorsten <quintfall@googlemail.com> writes:

>> Too bad you didn't give any concrete examples.
>
> The problem can be described easily: 
>
> problem-specific helper-funcions (some redundancy avoided)
> ,-----------------------------------------------------------
> | (defun main-function (args)                               
> | (let ((var (assoc :key1 args)))  ; extracting var once
> | ...                                                       
> | (helper-function1 ...) ; inside let using var             
> | (helper-function2 ...) ; inside let using var             
> | ))                                                        
> |                                                           
> | (defun helper-function1 ()                                
> | ...                                                       
> | )                                                         
> |                                                           
> | (defun helper-function2 ()                                
> | ...                                                       
> | )                                                         
> `-----------------------------------------------------------
>
> vs
>
> standalone helper-functions (but redundancy)
> ,-------------------------------------------------------------
> | (defun main-function (args)                                 
> | (let ((value (assoc :key1 args))   ; extracting var 1st time
> | ...                                                         
> | )                                                           
> | (helper-function1 ...) ; outside let                        
> | (helper-function2 ...) ; outside let                        
> | )                                                           
> |                                                             
> | (defun helper-function1 (args)                              
> | (let ((var (assoc :key1 args)))  ; extracting var 2nd time  
> | ...                                                         
> | ))                                                          
> |                                                             
> | (defun helper-function2 (args)                              
> | (let ((var (assoc :key1 args)))  ; extracting var 3rd time  
> | ...                                                         
> | ))                                                          
> `-------------------------------------------------------------

That's still very vague.  If `args' is some structure that has some
meaning and its parts belong together, say, an org entry, then it makes
sense to have the helper functions defined on that structure in order to
provide a consistent interface throughout the library.  And I wouldn't
tell multiple similar let-bindings duplicate code.

But again, that's only my very abstract point of view.  Given a concrete
example, my opinion might be completely upside-down. ;-)

Bye,
Tassilo

Re: Elisp programming style

[Eric Schulte]

Tassilo Horn <tassilo@member.fsf.org> writes:

> Thorsten <quintfall@googlemail.com> writes:
>
>>> Too bad you didn't give any concrete examples.
>>
>> The problem can be described easily: 
>>
>> problem-specific helper-funcions (some redundancy avoided)
>> ,-----------------------------------------------------------
>> | (defun main-function (args)                               
>> | (let ((var (assoc :key1 args)))  ; extracting var once
>> | ...                                                       
>> | (helper-function1 ...) ; inside let using var             
>> | (helper-function2 ...) ; inside let using var             
>> | ))                                                        
>> |                                                           
>> | (defun helper-function1 ()                                
>> | ...                                                       
>> | )                                                         
>> |                                                           
>> | (defun helper-function2 ()                                
>> | ...                                                       
>> | )                                                         
>> `-----------------------------------------------------------
>>
>> vs
>>
>> standalone helper-functions (but redundancy)
>> ,-------------------------------------------------------------
>> | (defun main-function (args)                                 
>> | (let ((value (assoc :key1 args))   ; extracting var 1st time
>> | ...                                                         
>> | )                                                           
>> | (helper-function1 ...) ; outside let                        
>> | (helper-function2 ...) ; outside let                        
>> | )                                                           
>> |                                                             
>> | (defun helper-function1 (args)                              
>> | (let ((var (assoc :key1 args)))  ; extracting var 2nd time  
>> | ...                                                         
>> | ))                                                          
>> |                                                             
>> | (defun helper-function2 (args)                              
>> | (let ((var (assoc :key1 args)))  ; extracting var 3rd time  
>> | ...                                                         
>> | ))                                                          
>> `-------------------------------------------------------------
>
> That's still very vague.  If `args' is some structure that has some
> meaning and its parts belong together, say, an org entry, then it makes
> sense to have the helper functions defined on that structure in order to
> provide a consistent interface throughout the library.  And I wouldn't
> tell multiple similar let-bindings duplicate code.
>

To clarify my earlier reply, I would agree with Tassilo above.  If args
is a global variable (declared with defvar or defconst) then using it
without accepting it as an argument is not a problem (and the emacs-lisp
compiler will not complain).

Best -- Eric

-- 
Eric Schulte
http://cs.unm.edu/~eschulte/

Re: Elisp programming style

[Thorsten]

Tassilo Horn <tassilo@member.fsf.org> writes:

> And I wouldn't tell multiple similar let-bindings duplicate code.

I have to get used to that, at first sight it looks like there is
redundancy everywhere. But its more from the Java perspective, from the
more functional perspective it makes sense.

cheers
-- 
Thorsten

Re: Elisp programming style

[Eric Schulte]

>
> The problem can be described easily: 
>
> problem-specific helper-funcions (some redundancy avoided)
> ,-----------------------------------------------------------
> | (defun main-function (args)                               
> | (let ((var (assoc :key1 args)))  ; extracting var once
> | ...                                                       
> | (helper-function1 ...) ; inside let using var             
> | (helper-function2 ...) ; inside let using var             
> | ))                                                        
> |                                                           
> | (defun helper-function1 ()                                
> | ...                                                       
> | )                                                         
> |                                                           
> | (defun helper-function2 ()                                
> | ...                                                       
> | )                                                         
> `-----------------------------------------------------------
>
> vs
>
> standalone helper-functions (but redundancy)
> ,-------------------------------------------------------------
> | (defun main-function (args)                                 
> | (let ((value (assoc :key1 args))   ; extracting var 1st time
> | ...                                                         
> | )                                                           
> | (helper-function1 ...) ; outside let                        
> | (helper-function2 ...) ; outside let                        
> | )                                                           
> |                                                             
> | (defun helper-function1 (args)                              
> | (let ((var (assoc :key1 args)))  ; extracting var 2nd time  
> | ...                                                         
> | ))                                                          
> |                                                             
> | (defun helper-function2 (args)                              
> | (let ((var (assoc :key1 args)))  ; extracting var 3rd time  
> | ...                                                         
> | ))                                                          
> `-------------------------------------------------------------
>

Hmmm, this looks suspiciously like the case in some Babel functions :)
in which we originally has instances of the first style and then had to
manually transition to the second.  IMO the first is very poor form, the
variables are technically "free variables" when defined in the helper,
and just through undocumented variable capture does the execution work
out correctly, this can lead to unpleasant bugs.

The means the helper may only be used when variables of certain names
are in scope.

If you do want to use a helper which uses variables in scope which
aren't passed as arguments (again just my opinion) you should defined
the helper function using `flet' *inside* of the main function and the
scope of the variables.

>
>
>> In general, if the helper function is only used in a single place and
>> isn't likely to be more generally useful, there might be no reason for
>> it to be a standalone function at all (other than modular programming,
>> which is always a good idea IMO).
>>
>> OTOH, if the helper function _is_ standalone and used in multiple
>> places, then there is no "redundancy", so I'm really not sure what you
>> mean.
>
> Is there a policy? Lets say, an org-library author thinks his helper
> functions will be of no use for nobody else and designs them
> non-standalone. A few years later somebody wants to write a similar
> library and trys to reuse some of the old helper functions, to no avail. 
>
> Would that be considered bad style from the original author, or is that
> up to personal choice and not considered a problem?
>

I don't believe we have an official canon of such rules, but personally
I would consider this to be bad style.  If the helper function is only
used in one main function then it should be defined using flet, if it is
used across multiple functions then the in-scope variables should be
passed as explicit arguments (preferably with names other than those
which it has in scope so you can be sure you are actually using the
arguments).

Finally, I believe the emacs-lisp compiler would complain about such
free variables.

Hope this helps, Best -- Eric

>
> cheers

-- 
Eric Schulte
http://cs.unm.edu/~eschulte/

Re: Elisp programming style

[Thorsten]

Eric Schulte <schulte.eric@gmail.com> writes:

Hi Eric, 

> Hmmm, this looks suspiciously like the case in some Babel functions :)

well ... ;)

> in which we originally has instances of the first style and then had to
> manually transition to the second.  IMO the first is very poor form, the
> variables are technically "free variables" when defined in the helper,
> and just through undocumented variable capture does the execution work
> out correctly, this can lead to unpleasant bugs.
>
> The means the helper may only be used when variables of certain names
> are in scope.
>
> If you do want to use a helper which uses variables in scope which
> aren't passed as arguments (again just my opinion) you should defined
> the helper function using `flet' *inside* of the main function and the
> scope of the variables.

That sounds like an operational prescription to me, and I think will
follow this advice in the future.

>> Would that be considered bad style from the original author, or is that
>> up to personal choice and not considered a problem?
>>
>
> I don't believe we have an official canon of such rules, but personally
> I would consider this to be bad style.  If the helper function is only
> used in one main function then it should be defined using flet, if it is
> used across multiple functions then the in-scope variables should be
> passed as explicit arguments (preferably with names other than those
> which it has in scope so you can be sure you are actually using the
> arguments).
>
> Finally, I believe the emacs-lisp compiler would complain about such
> free variables.
>
> Hope this helps, Best -- Eric

yes that helped, thanks

cheers
-- 
Thorsten

Re: Elisp programming style

[Tom Prince]

Perhaps 

,-----------------------------------------------------------
| (defun main-function (args)                               
| (let ((var (assoc :key1 args)))  ; extracting var once    
| ...                                                       
| (helper-function1 ... var ...) ; inside let using var     
| (helper-function2 ... var ...) ; inside let using var     
| ))                                                        
|                                                           
| (defun helper-function1 (var')                            
| ...                                                       
| )                                                         
|                                                           
| (defun helper-function2 (var')                            
| ...                                                       
| )                                                         
`-----------------------------------------------------------

or

,-------------------------------------------------------------
| (defun get-key1 (args) (assoc :key1 args))                  
| (defun main-function (args)                                 
| (let ((value (get-key1 args))   ; extracting var 1st time   
| ...                                                         
| )                                                           
| (helper-function1 ...) ; outside let                        
| (helper-function2 ...) ; outside let                        
| )                                                           
|                                                             
| (defun helper-function1 (args)                              
| (let ((value (get-key1 args))   ; extracting var 2nd time   
| ...                                                         
| ))                                                          
|                                                             
| (defun helper-function2 (args)                              
| (let ((value (get-key1 args))   ; extracting var 3rd time   
| ...                                                         
| ))                                                          
`-------------------------------------------------------------


I likely wouldn't suggest the second, unless get-key1 was actually
something more complicated than your example.

  Tom

Re: Elisp programming style

[Thorsten]

Tom Prince <tom.prince@ualberta.net> writes:

> Perhaps 
>
> ,-----------------------------------------------------------
> | (defun main-function (args)                               
> | (let ((var (assoc :key1 args)))  ; extracting var once    
> | ...                                                       
> | (helper-function1 ... var ...) ; inside let using var     
> | (helper-function2 ... var ...) ; inside let using var     
> | ))                                                        
> |                                                           
> | (defun helper-function1 (var')                            
> | ...                                                       
> | )                                                         
> |                                                           
> | (defun helper-function2 (var')                            
> | ...                                                       
> | )                                                         
> `-----------------------------------------------------------
>
> or
>
> ,-------------------------------------------------------------
> | (defun get-key1 (args) (assoc :key1 args))                  
> | (defun main-function (args)                                 
> | (let ((value (get-key1 args))   ; extracting var 1st time   
> | ...                                                         
> | )                                                           
> | (helper-function1 ...) ; outside let                        
> | (helper-function2 ...) ; outside let                        
> | )                                                           
> |                                                             
> | (defun helper-function1 (args)                              
> | (let ((value (get-key1 args))   ; extracting var 2nd time   
> | ...                                                         
> | ))                                                          
> |                                                             
> | (defun helper-function2 (args)                              
> | (let ((value (get-key1 args))   ; extracting var 3rd time   
> | ...                                                         
> | ))                                                          
> `-------------------------------------------------------------
>
>
> I likely wouldn't suggest the second, unless get-key1 was actually
> something more complicated than your example.

hmm ... I feel quite convinced now that having a standalone function is
worth a bit of code duplication, and I start do discover that the cl
package does have some nice functions (like flet). 

cheers
-- 
Thorsten

Re: Elisp programming style

[Tassilo Horn]

Thorsten <quintfall@googlemail.com> writes:

> I start do discover that the cl package does have some nice functions
> (like flet).

For code that's included in emacs (or wants to be), you should not
require the cl package at runtime.  But it's ok to require it at compile
time for using its macros [dolist, flet, ...].

Bye,
Tassilo
-- 
(What the world needs (I think) is not
      (a Lisp (with fewer parentheses))
      but (an English (with more.)))
Brian Hayes, http://tinyurl.com/3y9l2kf