[erlang-questions] is this a well written function

[Richard A. O'Keefe]

On 11/02/2015, at 8:45 pm, Roelof Wobben <r.wobben@REDACTED> wrote:
> -export([display_numbers/1]).
> 
> %%  displays(N) displays the numbers from  1,2,...+N when N is a non-negative integer.
> %%  It is not defined for other arguments.
> %%  When N is a non-negative number, a helper function is called so it prints out
> %%  the numbers in the right order. This is a exercise from the Erlang Programming book
> %%  where I have to practice side-effects.

The comment disagrees with the code.
The comment says the function call is 'displays(_)','
but the code says it is 'display_numbers(_)'.

The comment disagrees with the code.
The comment says "when N is a NON-NEGATIVE INTEGER",
but the code insists on a STRICTLY POSITIVE number of
any kind.  So the comment would accept 0 and reject 1.2,
while the code rejects 0 and accepts 1.2.

> display_numbers(Number) when Number > 0 ->
>  display_numbers_loop(Number, 1).

You would do better to stick with the comment,
and to count UP from 0, so that the control argument
actually means something: it counts how many numbers
already printed.  Also, control arguments are
conventionally written first.

display_numbers(N) when is_integer(N), N >= 0 ->  % 1; A,B
    display_numbers_loop(0, N).                   % 2; C

%   In display_numbers_loop(N0, N), N is how many numbers
%   we want to print altogether, and N0 is how many
%   numbers we have already printed.

display_numbers_loop(N, N) ->                     % 3; C
    ok;                                           % 4; F
display_numbers_loop(N0, N) ->                    % 5;
    N1 = N0 + 1,                                  % 6; C,D
    io:format("Number: ~p~n", [N1]),              % 7; D
    display_numbers_loop(N1, N).                  % 8; 


> %%  When the control argument)(second argument) is equal to the number
> %%  the user has given, the end is reached and the last number is printed
> display_numbers_loop(Number, Number) ->
>  io:format("Number:~p~n",[Number]);
> 
> %%  When the contro argument(second argument) is not equal to the number
> %5  the user has given the control argument is increased by one and the
> %%  display_numbers_loop function is called again with the new arguments.
> display_numbers_loop(Number, Current) ->
>  io:format("Number:~p~n",[Current]),
>  display_numbers_loop(Number, Current +1 ).

You have two copies of the io:format/2 call.
That should set off alarm bells in your mind.
(Martin Fowler uses the term "code smells".)
If you want to do "something" N times, then
"something" should normally be written once
and only once.

Think about it:  it's pretty ugly to omit the space after
the colon in the format.  We only want to have to fix that
just once.  If it's written twice, it's way too easy to
fix one copy and not the other.

If you were doing this in C, you would have

void display_numbers(int N) {             // 1; A
    assert(N >= 0);                       // 1; B
    for (int I = 0; I < N; I++) {         // 2,3,5,8; C
        printf("Number: %d\n", I+1);      // 7; D
    }                                     // 8; E
}                                         // 4; F

I've numbered the Erlang lines with integers and the C lines
with letters to show the correspondence of the parts.

Oh yeah, one more thing.  Who is "the user"?  A "user" is a
human being, but functions are usually called by other functions.

Why did I start the loop at 0, not 1?
Generally speaking, counting "what have I already done"
seems to result in many fewer errors than "what is the
next thing to do".  I originally learned Fortran and COBOL
and Algol where the index origin is (Fortran, COBOL) or
is conventionally (Algol) 1.  In languages like C and
Lisp where the index origin is 0, I make so many fewer
off-by-one errors that it just is not funny.  (Yes, I do
find it regrettable that Erlang indexes tuples from 1.)
To be honest, Smalltalk's use of index origin 1 is a
constant pain in the (your choice of body part).

You might want to read "Why numbering should start at zero."
https://www.cs.utexas.edu/users/EWD/transcriptions/EWD08xx/EWD831.html

Oh double yeah.  There is a general translation from a C
'for' loop to a functional (tail) recursion.

f(...) {
    g(...);
    for (T x = i(...); p(x, ...); x = u(x, ...) {
        b(x, ...);
    }
    c(...);
}

turns into

f(...) ->
    g(...),
    f_loop(i(...), ...).

f_loop(X, ...) when p(X, ...) ->
    b(X, ...),
    f_loop(u(X, ...), ...);
f_loop(_, ...) ->
    c(...).

Things get a little more tricky when p(X, ...) doesn't fit the
restrictions of a guard, and I've said nothing about variables
being updated in the loop, but this is how you start.

In a strict functional language with strong static types, like
SML, I'd expect the C version and the functional version to
result in pretty much the same machine code.