magic numbers

[bairui]

Code Select Expand

(error-event (fn ()
    (if (= 4 ((last-error) 0))
        (println "ERR:" (last (last-error 4)))
        (println ((last-error) 1)))))
[/quote]

For a while now I have been unconsciously squirming in my seat when reading over code like this. This morning I became aware of that discomfort and cried in alarm: magic numbers!



To the trained webber this might read transparently lucid; succinct even. But to the novice seeing this for the first time or the junior revisiting six month (week? day?!) old code, these opaque little numbers muddy the cognitive stream, making it nearly impossible to reason about the code until the manual has been consulted.



, where ((last-error) 0) and ((last-error) 1) retrieve the number-only and string-only, respectively.



Ok. That's not too bad. Two things to learn there and you're set for life:

1. newLISP's awesome implicit list indexing;

2. (last-error) returns such an ordered pair



fine, but what about those 4's...?

Code Select Expand

(if (= 4 ((last-error) 0))

Is now approachable: If the last error number is 4...

But, what is error 4?

Another quick test:

Code Select Expand

(last-error 4)

yielded:

Code Select Expand

(4 "problem accessing file")

ah... light-bulb



While the solution to the mystical zero and one numbers was trivial, the same approach will not work here -- it is not plausible to memorise the table of errors.



Error Codes[/url] handy;



2. Have constants representing error numbers:

Code Select Expand

(last-error ERR_FILE_ACCESS)
or the more cryptic:

Code Select Expand

(last-error EFACC)

Note: I am not literally suggesting either actual constant here -- I presume actual thought would be given to such a scheme.



[size=120]tl;dr[/size]



What is the recommended way to reduce magic numbers in our newLISP code?

[cormullion]

You could do something like this:

Code Select Expand

(context 'Error)
(for (i 1 120)
   (set 'new-symbol (sym (replace "[^A-z]|\[\]" (last (last-error i)) "-" 0)))
   (constant (sym new-symbol) i))
(context MAIN)
then you could refer to errors by name in your code:

Code Select Expand

(error-event (fn ()
    (if (= Error:problem-accessing-file ((last-error) 0))
        (println "The error was: " (last (last-error 4)))
        (println ((last-error) 1)))))

[bairui]

Thanks for playing along, Cormulion.



Your solution is certainly the easiest of the named-symbols implementations. On the down side, it produces fairly verbose symbols.



Another question I had about the code: two fours?



Is it possible for the state of (last-error) to change between testing for Error:problem-accessing-file and the (println ...) reporting it? If not, the code can be simplified to:

Code Select Expand

(error-event (fn ()
    (if (= Error:problem-accessing-file ((last-error) 0))
        (println "The error was: " (last (last-error)))
        (println ((last-error) 1)))))

no?

[Lutz]

Is it possible for the state of (last-error) to change between testing for Error:problem-accessing-file and the (println ...) reporting it?

In your example, the result of (last-error) will not change.



In general: (last-error) will report any error or ocurring or caused by (throw-error ...). Even when using catch it will first setup for (last-error):

Code Select Expand

> (throw-error "wrong!")

ERR: user error : wrong!
> (last-error)
(58 "ERR: user error : wrong!")

> (catch (foo bar) 'result)
nil
> result
"ERR: invalid function in function catch : (foo bar)"
> (last-error)
(24 "ERR: invalid function in function catch : (foo bar)")
>

> (catch (throw-error "BAD!!!") 'result)
nil
> result
"ERR: user error : BAD!!!"
> (last-error)
(58 "ERR: user error : BAD!!!")
>


[bairui]

Good to know, Lutz, thanks.



So... care to weigh in on the magic-number side?

This phenomenon exists in several places throughout the newLISP API. Many functions take an optional integer parameter that is an OR compositie of individual option values. As bare numbers, this makes reading code quite difficult. Perl (and awk before/after it) went through these criticisms and came out with the English package for tree hugging hippies like myself. Am I the only one here who feels this might benefit the newLISP landscape?

[Lutz]

In principle, I agree, but defining symbols for all 'Magic number constants' used in newLISP would grow the small minimalistic newLISP executable and memory footprint.



I suggest defining the constants used for a specific script at the beginning, or in a company multi-developer situation create a module containing all constants used. That module would have to be used by all developers on the project.



These are the functions for which constants could be defined:



Error handling: last-error, net-error, sys-error



Regular expressions: directory, ends-with, find, find-all, member, parse, regex, regex-comp, replace, search, starts-with



XML: xml-parse (also define composites, e.g. SXML = 31 (+ 1 2 4 8 16) )



Multiprocessing: destroy, signal, process (on Windows), wait-pid



Application exit: exit



Directory masks: make-dir



Localization: set-locale



Timer: timer

[bairui]

Thanks for the list of places to cover, Lutz.



Adding adhoc constants to individual scripts will lead to an inevitable mess of misspellings. The idea of a company-wide constants module is good -- and such a project would be the beginnings of a globally used Constants.lsp or English.lsp or SomeSuch.lsp, allowing so inclined devs to 'turn on English constants' a la Perl.

[Lutz]

Forgot about sys-error and net-error, added both to the original post. 'sys-error' is huge (100+ on OSX) and operating system dependent. The number and description strings are retrieved from the OS and just passed on to newLISP. Only last-error and net-error are defined inside newLISP.



Constants for error handling are probably the least used of all. So one would start with the other categories, which are also a lot smaller, the biggest one being options for regular expressions.

[conan]

For a while now I have been unconsciously squirming in my seat when reading over code like this. This morning I became aware of that discomfort and cried in alarm: magic numbers!



To the trained webber this might read transparently lucid; succinct even. But to the novice seeing this for the first time or the junior revisiting six month (week? day?!) old code, these opaque little numbers muddy the cognitive stream, making it nearly impossible to reason about the code until the manual has been consulted.

I agree. In particular I think the benefit of having verbose-constants for each code is not on the writing side, but on the reading side. Because when you're writing code you'll probably end up checking the manual anyway. But when you read code, you want to understand what's happening right away without the need to check a manual. So better to have verbose constants.

In principle, I agree, but defining symbols for all 'Magic number constants' used in newLISP would grow the small minimalistic newLISP executable and memory footprint.

I agree with that too. And also I like cormullion's approach of building the constants programmatically.



I thought this would be a nice and easy project for a person like me, coming again to newLISP after nearly a year, and having forgotten almost everything about the language.



So I created this project: https://github.com/conan-lugmen/newlisp-constants">//https://github.com/conan-lugmen/newlisp-constants



It's based on cormullion idea of building constants programmatically, though I used predefined sets of values (taken from the manual) to build NewlispErrorCodes and NetErrorCodes.



I think it would be a nice addition to newLISP to slightly modify last-error and net-error to return the full list of possible codes so the modules could build all constants programatically.



Also if sys-error codes gets built into newlisp at compilation time, then it can be modified too that way.



I think it wouldn't be much overhead to add an if to test for the given parameter and if it's, say... a string saying "show all" or maybe a negative integer, then return the full list of codes for that function.



What do you say Lutz?



Anyway, the Constants module can be used to build any kind of constants, though I agree with bairui that it would be nice to have some set of predefined/standard ones.



I made it verbose, but users can always do a:

Code Select Expand

(constant 'C Constants)

Anyway, I had fun making this, whether useful or not, I would like to request for comments on ways to improve code since I'm relearning everything.



In particular, I'm interested in knowing why I couldn't use:

Code Select Expand

(constant 'prefix "XXX")

I got "ERR: symbol not in current context in function constant : prefix" when I use it like above.

[Lutz]

prefix is a built-in function and should not be redefined without an appropriate reason. Like all built-in functions and other symbols, it is global and protected. The constant function allows you to redefine even those, but only when in the same context / namespace. This is where the error message comes from.



http://www.newlisp.org/downloads/newlisp_manual.html#prefix">http://www.newlisp.org/downloads/newlis ... tml#prefix">http://www.newlisp.org/downloads/newlisp_manual.html#prefix



http://www.newlisp.org/downloads/newlisp_manual.html#constant">http://www.newlisp.org/downloads/newlis ... l#constant">http://www.newlisp.org/downloads/newlisp_manual.html#constant

[conan]

prefix is a built-in function and should not be redefined without an appropriate reason.

Ooops! I just wanted an innocent constant, not trying to redefine anything.



Didn't want to use uppercase characters to avoid a possible collision with the constants that will be defined. I'll change it.