[erlang-questions] is this a well written function
Richard A. O'Keefe
ok@REDACTED
Fri Feb 13 04:30:25 CET 2015
Time to get onto some of the fine detail.
(1) The main comment sometimes says "integer" and sometimes says "number".
This is at best confusing. All integers are numbers, but not all
numbers are integers.
(2) "The Erlang Programming book" is not a helpful reference.
Which book? Is the book called "Erlang Programming"? Then
a better comment would be
This is exercise <which> from page <what> of
"Programming Erlang" by <authors>
For example, your b_and/2 &c exercise should have been documented as
%%% This module is my solution to exercise 2-3 on page 44 of
%%% "Programming Erlang" by Cesarini and Thomson.
By the way, that is a really fine book. The wording of some of
the exercises could be improved. For example, exercise 3-2 says
"format" when it means "form". The first time I saw this book,
I was so impressed by the rest of the text that I paid no attention
to the exercises.
(3) So let's start with a master comment for the module.
%%% This module is my solution to exercise 3-3 "Side Effects" on
%%% page 83 of "Programming Erlang" by Cesarini and Thomson.
(4) Let's note the wording of the exercise.
"Write a function that PRINTS out the INTEGERS between 1 and N."
"Write a function that PRINTS out the EVEN INTEGERS between 1 and N."
It is wise to stick with the words you are given. Someone who knows
that you are supposed to "print integers" is going to find
'print_integers' *instantly* understandable, while
'display_numbers' is going to take some decoding: what is the
difference between displaying and printing, what is the difference
between integers and numbers (is your function meant to handle
floats as well?)
So we should continue the module like this:
-module(exercise_3_2).
-export([
print_even_integers/1,
print_integers/1
]).
%% "a function that prints out the integers between 1 and N."
print_integers(N) when is_integer(N), N >= 0 ->
...
%% "a function that prints out the even integers between 1 and N."
print_even_integers(N) when is_integer(N), N >= 0 ->
...
In the spirit of sticking with the words you are given, the text of
the exercise consistently calls the upper bound N. You could make
a case for using a *MORE* informative name, such as Upper_Bound.
But using Number is not a good idea. It is *different* from the
name used in the exercise, so the reader constantly has to keep in
mind "Number really means N" instead of the much simpler "N means N",
and it does not *help* in any way that N does not. It is in fact
*less* helpful than N, because there has been a convention since
before Fortran (that since, since before the late 1950s) that N is
to be used only for integers.
(5) Consistency again.
>
> display_numbers(Number) when is_integer(Number), Number > 0 ->
> display_numbers_loop(0, Number ).
You have no space before the right parenthesis in the head,
and that is GOOD.
You one one space before the right parenthesis in the body,
which I find ugly, but some people think it's a good idea.
What nobody thinks is a good idea is mixing BOTH styles in
the same function!
Like everything I've mentioned in this message so far, this has
nothing to do with Erlang. Some people consistently write
f(X). Some people consistently write f( X ). Some people
consistently write f(X ) or consistently write f( X), both of
which look horrible to me. Feel free to choose a style
without paying any heed to what I like or dislike, but have
*reasons* for your choice and be consistent.
Cesarini and Thomson's style is consistently
- no space after ( in a function call
- no space before ) in a function call.
>
> %% When the control argument)(second argument)
(6) This is a classic "incomplete edit" problem. I found one of those in
my own code two days ago. I was ashamed of it. A major refactoring
had drastically simplified a large file, but some old code that
wasn't needed any more was still being called, and in the new context,
doing bad things. The control argument is in fact the second.
It would be even better to write
%% print_integers(N0, N) writes the rest of the integers 1 .. N
%% after the integers 1 .. N0 have already been printed.
> display_numbers_loop(Current, Number) ->
> io:format("Number:~p~n",[Current + 1]),
> display_numbers_loop(Current +1, Number ).
(7) Consistency again. There are three argument-separating commas.
Two of them are followed by spaces but one is not.
There are two addition operators. One of them has spaces on
both sides, but the other only on one side.
Consistency in a layout style doesn't get noticed.
Inconsistency in a layout style slows your readers down while
they try to figure out whether a difference *means* something
or is an accident.
By the way, exercise 3-3 includes
Hint: use guards.
You are using guards to check the argument N, which is fine,
but that's not what the hint is pointing you to, and if you had
started with print_even_integers/1 first, you would have noticed.
Let's be clever.
To print all the integers between 1 and N is to
print all the integers L, L+1, L+2, ... U
where L = 1 and U = N.
To print the even integers between 1 and N is to
print all the integers L, L+2, L+4, ... U
where L = 2 and U = N.
If we had a function
print_integers(Lower_Bound, Upper_Bound, Increment)
that did the obvious thing, then we could write
print_integers(1, N) when is_integer(N), N >= 0 ->
print_integers(1, N, 1).
print_even_integers(1, N) when is_integer(N), N >= 0 ->
print_even_integers(2, N, 2).
So all we need is print_integers/3. And _that_ is basically
display_numbers_loop/2, except that the constant 1 is replaced
by the Increment argument, and the test for having printed all
the numbers uses a guard Lower_Bound > Upper_Bound instead of
a pattern match.
Why a guard? Because print_even_integers(5) is going to stop
when the next number to print is 6; it will never be 5.
(8) AUTHORITIES MAY DISAGREE.
In exercise 3-4 on page 84 of Cesarini and Thomson, they
tell you to write flatten/1 and hint that you should
use concatenate/1 in your answer.
It is indeed *possible* to use concatenate/1 in the answer,
but it is much simpler and an order of magnitude more
efficient not to. I suspect that Cesarini and Thomson
are trying to get you to
(a) do something with a tree structure, not just a sequence
(b) give you some more practice in using building blocks
you've just written
(c) give you some practice in seeing [_|_] as a binary tree
constructor, not specifically a sequence constructor.
I can't at the moment see a better exercise they could have
written to serve those purposes, but in actual programming
practice, using concatenate/1 in flatten/1 is as bad as
and for much the same reason as doing string concatenation
in a loop in Java.
More information about the erlang-questions
mailing list