is this a well written function

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

is this a well written function

Roelof Wobben-2
Hello,

I tried to solve the exercise where I have to calculate the outcome of 1
... n

So I have this :

-module(sum_recursion).

-export([sum/1]).

% when the number is zero the outcome will also be zero
sum(0) ->
   0;

% when the output is a number higher then 0 , the acc function will be
called.
sum(Number) -> sum_acc(Number, 0, 0 ).

% when the numbers are equal then the end is reached and the last item
is added to the acc variable,
% the acc variable hold the value of the sum as it is calculated
sum_acc(Endnumber,  Endnumber, Acc) ->
   Acc + Endnumber ;

% when the numbers are not equal then the end is not reached so the
function is again called with the original number,
% the loop is increased by 1 and the acc function hold the value of the
sum as it is calculated.
sum_acc(Endnumber, Number, Acc) ->
   sum_acc(Endnumber, Number + 1, Acc + Number).


Now I wonder if this is well written Erlang ?

Roelof

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Bengt Kleberg
I like it.
It could be enhanced with a test that _input_ Number is higher _than_ 0 in:
% when the output is a number higher then 0 , the acc function will be
called.
sum(Number) -> sum_acc(Number, 0, 0 ).


bengt

On 02/09/2015 10:38 AM, Roelof Wobben wrote:

> Hello,
>
> I tried to solve the exercise where I have to calculate the outcome of
> 1 ... n
>
> So I have this :
>
> -module(sum_recursion).
>
> -export([sum/1]).
>
> % when the number is zero the outcome will also be zero
> sum(0) ->
>   0;
>
> % when the output is a number higher then 0 , the acc function will be
> called.
> sum(Number) -> sum_acc(Number, 0, 0 ).
>
> % when the numbers are equal then the end is reached and the last item
> is added to the acc variable,
> % the acc variable hold the value of the sum as it is calculated
> sum_acc(Endnumber,  Endnumber, Acc) ->
>   Acc + Endnumber ;
>
> % when the numbers are not equal then the end is not reached so the
> function is again called with the original number,
> % the loop is increased by 1 and the acc function hold the value of
> the sum as it is calculated.
> sum_acc(Endnumber, Number, Acc) ->
>   sum_acc(Endnumber, Number + 1, Acc + Number).
>
>
> Now I wonder if this is well written Erlang ?
>
> Roelof
>
> _______________________________________________
> erlang-questions mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-questions

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Masklinn
In reply to this post by Roelof Wobben-2

On 2015-02-09, at 10:38 , Roelof Wobben <[hidden email]> wrote:

> Hello,
>
> I tried to solve the exercise where I have to calculate the outcome of 1 ... n
>
> So I have this :
>
> -module(sum_recursion).
>
> -export([sum/1]).
>
> % when the number is zero the outcome will also be zero
> sum(0) ->
>  0;
>
> % when the output is a number higher then 0 , the acc function will be called.
> sum(Number) -> sum_acc(Number, 0, 0 ).
>
> % when the numbers are equal then the end is reached and the last item is added to the acc variable,
> % the acc variable hold the value of the sum as it is calculated
> sum_acc(Endnumber,  Endnumber, Acc) ->
>  Acc + Endnumber ;
>
> % when the numbers are not equal then the end is not reached so the function is again called with the original number,
> % the loop is increased by 1 and the acc function hold the value of the sum as it is calculated.
> sum_acc(Endnumber, Number, Acc) ->
>  sum_acc(Endnumber, Number + 1, Acc + Number).
>
>
> Now I wonder if this is well written Erlang ?

I'd tend to write something like this instead:

sum(End) -> sum_acc(0, 0, End).

sum_acc(Acc, _, 0) ->
    Acc;
sum_acc(Acc, Current, Rem) ->
    sum_acc(Acc + Current, Current + 1, Rem - 1).
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Roelof Wobben-2
Masklinn schreef op 9-2-2015 om 10:51:

> On 2015-02-09, at 10:38 , Roelof Wobben <[hidden email]> wrote:
>
>> Hello,
>>
>> I tried to solve the exercise where I have to calculate the outcome of 1 ... n
>>
>> So I have this :
>>
>> -module(sum_recursion).
>>
>> -export([sum/1]).
>>
>> % when the number is zero the outcome will also be zero
>> sum(0) ->
>>   0;
>>
>> % when the output is a number higher then 0 , the acc function will be called.
>> sum(Number) -> sum_acc(Number, 0, 0 ).
>>
>> % when the numbers are equal then the end is reached and the last item is added to the acc variable,
>> % the acc variable hold the value of the sum as it is calculated
>> sum_acc(Endnumber,  Endnumber, Acc) ->
>>   Acc + Endnumber ;
>>
>> % when the numbers are not equal then the end is not reached so the function is again called with the original number,
>> % the loop is increased by 1 and the acc function hold the value of the sum as it is calculated.
>> sum_acc(Endnumber, Number, Acc) ->
>>   sum_acc(Endnumber, Number + 1, Acc + Number).
>>
>>
>> Now I wonder if this is well written Erlang ?
> I'd tend to write something like this instead:
>
> sum(End) -> sum_acc(0, 0, End).
>
> sum_acc(Acc, _, 0) ->
>      Acc;
> sum_acc(Acc, Current, Rem) ->
>      sum_acc(Acc + Current, Current + 1, Rem - 1).
> _______________________________________________
> erlang-questions mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-questions
>


Thanks both,

This is a version with a guard to take care that n is greater then 0

Masklinn,

I will study your version to see how you see when it's is the end.

Roelof

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Roelof Wobben-2
Roelof Wobben schreef op 9-2-2015 om 14:39:
> sum(End) -> sum_acc(0, 0, End).
>
> sum_acc(Acc, _, 0) ->
>      Acc;
> sum_acc(Acc, Current, Rem) ->
>      sum_acc(Acc + Current, Current + 1, Rem - 1).


A new try after some remarks :

-module(sum_recursion).

-export([sum/1]).

% when the number is zero the outcome will also be zero
sum(0) ->
   0;

% when a number is greater then 0 , call the helper function.
sum(End) when End > 0 ->
   sum_acc(0, End).

% When End is equal to zero all the numbers are added
sum_acc(Acc, 0) ->
      Acc;

% when end is not zero there are more numbers to be added.
sum_acc(Acc, End) ->
      sum_acc(Acc + End, End - 1).

Roelof
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Martin Koroudjiev
It's perfect, just note that you will get "no function clause" exception
if you pass negative number to the function which is probably desired
result.

Martin

On 2/9/2015 4:11 PM, Roelof Wobben wrote:

> Roelof Wobben schreef op 9-2-2015 om 14:39:
>> sum(End) -> sum_acc(0, 0, End).
>>
>> sum_acc(Acc, _, 0) ->
>>      Acc;
>> sum_acc(Acc, Current, Rem) ->
>>      sum_acc(Acc + Current, Current + 1, Rem - 1).
>
>
> A new try after some remarks :
>
> -module(sum_recursion).
>
> -export([sum/1]).
>
> % when the number is zero the outcome will also be zero
> sum(0) ->
>   0;
>
> % when a number is greater then 0 , call the helper function.
> sum(End) when End > 0 ->
>   sum_acc(0, End).
>
> % When End is equal to zero all the numbers are added
> sum_acc(Acc, 0) ->
>      Acc;
>
> % when end is not zero there are more numbers to be added.
> sum_acc(Acc, End) ->
>      sum_acc(Acc + End, End - 1).
>
> Roelof
> _______________________________________________
> erlang-questions mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-questions

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Roelof Wobben-2
I know , the exercise is stated this :

Write a function sum/1which, given a positive integer N, will return the
sum of all the
integers between 1 and N.

Roelof


Martin Koroudjiev schreef op 9-2-2015 om 15:32:

> It's perfect, just note that you will get "no function clause" exception
> if you pass negative number to the function which is probably desired
> result.
>
> Martin
>
> On 2/9/2015 4:11 PM, Roelof Wobben wrote:
>> Roelof Wobben schreef op 9-2-2015 om 14:39:
>>> sum(End) -> sum_acc(0, 0, End).
>>>
>>> sum_acc(Acc, _, 0) ->
>>>       Acc;
>>> sum_acc(Acc, Current, Rem) ->
>>>       sum_acc(Acc + Current, Current + 1, Rem - 1).
>>
>> A new try after some remarks :
>>
>> -module(sum_recursion).
>>
>> -export([sum/1]).
>>
>> % when the number is zero the outcome will also be zero
>> sum(0) ->
>>    0;
>>
>> % when a number is greater then 0 , call the helper function.
>> sum(End) when End > 0 ->
>>    sum_acc(0, End).
>>
>> % When End is equal to zero all the numbers are added
>> sum_acc(Acc, 0) ->
>>       Acc;
>>
>> % when end is not zero there are more numbers to be added.
>> sum_acc(Acc, End) ->
>>       sum_acc(Acc + End, End - 1).
>>
>> Roelof
>> _______________________________________________
>> erlang-questions mailing list
>> [hidden email]
>> http://erlang.org/mailman/listinfo/erlang-questions
> _______________________________________________
> erlang-questions mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-questions
>

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Fred Hebert-2
In reply to this post by Roelof Wobben-2
On 02/09, Roelof Wobben wrote:
> A new try after some remarks :
>
> % When End is equal to zero all the numbers are added
> sum_acc(Acc, 0) ->
>      Acc;

The only criticism I'd have is that usually, Erlang programmers will put
the accumulator last in the list of arguments, and the variable you
iterate on in first positions.

So the function clause above would be

    sum_acc(0, Acc) ->
        Acc;

Regards,
Fred.
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Robert Wilkinson
In reply to this post by Roelof Wobben-2
On Mon, Feb 09, 2015 at 03:38:10PM +0100, Roelof Wobben wrote:
> I know , the exercise is stated this :
>
> Write a function sum/1which, given a positive integer N, will return
> the sum of all the
> integers between 1 and N.
>
> Roelof

If that is the problem, then the most effective way is to sum the
Arithmetic progression?

n * (n + 1) / 2 is the sum of an AP (IIRC).

so

sum(N) -> (((N + 1) * N) / 2) .

which will be much faster than recursion ?

Bob

> Martin Koroudjiev schreef op 9-2-2015 om 15:32:
> >It's perfect, just note that you will get "no function clause" exception
> >if you pass negative number to the function which is probably desired
> >result.
> >
> >Martin
> >
> >On 2/9/2015 4:11 PM, Roelof Wobben wrote:
> >>Roelof Wobben schreef op 9-2-2015 om 14:39:
> >>>sum(End) -> sum_acc(0, 0, End).
> >>>
> >>>sum_acc(Acc, _, 0) ->
> >>>      Acc;
> >>>sum_acc(Acc, Current, Rem) ->
> >>>      sum_acc(Acc + Current, Current + 1, Rem - 1).
> >>
> >>A new try after some remarks :
> >>
> >>-module(sum_recursion).
> >>
> >>-export([sum/1]).
> >>
> >>% when the number is zero the outcome will also be zero
> >>sum(0) ->
> >>   0;
> >>
> >>% when a number is greater then 0 , call the helper function.
> >>sum(End) when End > 0 ->
> >>   sum_acc(0, End).
> >>
> >>% When End is equal to zero all the numbers are added
> >>sum_acc(Acc, 0) ->
> >>      Acc;
> >>
> >>% when end is not zero there are more numbers to be added.
> >>sum_acc(Acc, End) ->
> >>      sum_acc(Acc + End, End - 1).
> >>
> >>Roelof
> >>_______________________________________________
> >>erlang-questions mailing list
> >>[hidden email]
> >>http://erlang.org/mailman/listinfo/erlang-questions
> >_______________________________________________
> >erlang-questions mailing list
> >[hidden email]
> >http://erlang.org/mailman/listinfo/erlang-questions
> >
>
> _______________________________________________
> erlang-questions mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-questions
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Roelof Wobben-2
In reply to this post by Fred Hebert-2
Fred Hebert schreef op 9-2-2015 om 16:18:

> On 02/09, Roelof Wobben wrote:
>> A new try after some remarks :
>>
>> % When End is equal to zero all the numbers are added
>> sum_acc(Acc, 0) ->
>>       Acc;
> The only criticism I'd have is that usually, Erlang programmers will put
> the accumulator last in the list of arguments, and the variable you
> iterate on in first positions.
>
> So the function clause above would be
>
>      sum_acc(0, Acc) ->
>          Acc;
>
> Regards,
> Fred.
>

Thanks,

Changed it like this :

-module(sum_recursion).

-export([sum/1]).

% when the number is zero the outcome will also be zero
sum(0) ->
   0;

% when a number is greater then 0 , call the helper function.
sum(End) when End > 0 ->
   sum_acc(End,0).

% When End is equal to zero all the numbers are added
sum_acc(0,Acc) ->
      Acc;

% when end is not zero there are more numbers to be added.
sum_acc(End, Acc) ->
      sum_acc(End - 1, Acc + End).

Roelof

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Richard A. O'Keefe-2
In reply to this post by Roelof Wobben-2

On 9/02/2015, at 10:38 pm, Roelof Wobben <[hidden email]> wrote:

> Hello,
>
> I tried to solve the exercise where I have to calculate the outcome of 1 ... n

The outcome of doing what?

So I have this :
>
> -module(sum_recursion).

This is a poor name.  From the outside, you cannot tell how the sum is
computed, and you probably should not care.  If you are doing exercises
from a book, it is better to name your modules for the exercises,
e.g. chapter_2_exercise_4.
>
> -export([sum/1]).
>
> % when the number is zero the outcome will also be zero
> sum(0) ->
>  0;
>
> % when the output is a number higher then 0 , the acc function will be called.
> sum(Number) -> sum_acc(Number, 0, 0 ).

Your comments must be
(1) true
(2) useful.

Ad (1), there is no 'acc' function, so the comment is untrue.
Ad (2), the comment is not USEFUL.

What you are missing is a comment that says WHAT THE FUNCTION IS ABOUT.
Something like

%%  sum(N) answers 1+2+...+N when N is a non-negative integer.
%%  It is not defined for other arguments.
%%  The implementation uses tail recursion, where the
%%  control argument counts up from 0 to N and the
%%  accumulator argument holds the running prefix of the sum.

These observations on comments have nothing to do with Erlang.
No matter what programming language you are using, your comments
should be true, useful, and not just paraphrase the code.

>
> % when the numbers are equal then the end is reached and the last item is added to the acc variable,
> % the acc variable hold the value of the sum as it is calculated

This comment is not good.  You have *three* numbers.  Which two of them are supposed to be
equal?  The phrase "junk comment" is generally used for comments that simply paraphrase the
code.

In Erlang, you CANNOT "add to the acc variable" (which is Acc, not acc).
Once an Erlang variable has a value, that is its value for as long as it exists.

Good code for this might be

sum(N) when is_integer(N), N >= 0 ->
    (N * (N+1)) // 2.


_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Martin Koroudjiev

> sum(N) when is_integer(N), N >= 0 ->
>     (N * (N+1)) // 2.
>

Should be with a single forward slash. Also the OP should know that this
function will return a float.
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Roelof Wobben-2
In reply to this post by Richard A. O'Keefe-2
Richard A. O'Keefe schreef op 11-2-2015 om 5:26:

> On 9/02/2015, at 10:38 pm, Roelof Wobben <[hidden email]> wrote:
>
>> Hello,
>>
>> I tried to solve the exercise where I have to calculate the outcome of 1 ... n
> The outcome of doing what?
>
> So I have this :
>> -module(sum_recursion).
> This is a poor name.  From the outside, you cannot tell how the sum is
> computed, and you probably should not care.  If you are doing exercises
> from a book, it is better to name your modules for the exercises,
> e.g. chapter_2_exercise_4.
>> -export([sum/1]).
>>
>> % when the number is zero the outcome will also be zero
>> sum(0) ->
>>   0;
>>
>> % when the output is a number higher then 0 , the acc function will be called.
>> sum(Number) -> sum_acc(Number, 0, 0 ).
> Your comments must be
> (1) true
> (2) useful.
>
> Ad (1), there is no 'acc' function, so the comment is untrue.
> Ad (2), the comment is not USEFUL.
>
> What you are missing is a comment that says WHAT THE FUNCTION IS ABOUT.
> Something like
>
> %%  sum(N) answers 1+2+...+N when N is a non-negative integer.
> %%  It is not defined for other arguments.
> %%  The implementation uses tail recursion, where the
> %%  control argument counts up from 0 to N and the
> %%  accumulator argument holds the running prefix of the sum.
>
> These observations on comments have nothing to do with Erlang.
> No matter what programming language you are using, your comments
> should be true, useful, and not just paraphrase the code.
>
>> % when the numbers are equal then the end is reached and the last item is added to the acc variable,
>> % the acc variable hold the value of the sum as it is calculated
> This comment is not good.  You have *three* numbers.  Which two of them are supposed to be
> equal?  The phrase "junk comment" is generally used for comments that simply paraphrase the
> code.
>
> In Erlang, you CANNOT "add to the acc variable" (which is Acc, not acc).
> Once an Erlang variable has a value, that is its value for as long as it exists.
>
> Good code for this might be
>
> sum(N) when is_integer(N), N >= 0 ->
>      (N * (N+1)) // 2.
>
>

Thanks for all the remarks. I use recursion because the exercises from
the Erlang programming book asked me to use it.

Here another try from another exercise from that book.


-module(side_effects).

-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.
display_numbers(Number) when Number > 0 ->
   display_numbers_loop(Number, 1).

%%  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 ).

Roelof


_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Richard A. O'Keefe-2
In reply to this post by Martin Koroudjiev

On 11/02/2015, at 8:11 pm, Martin Koroudjiev <[hidden email]> wrote:

>
>> sum(N) when is_integer(N), N >= 0 ->
>>    (N * (N+1)) // 2.
>>
>
> Should be with a single forward slash.

Sorry.  I use too many programming languages, and notations for integer
division are just pointlessly diverse.  (And since Erlang syntax was
copied from Prolog, it is NOT helpful that it doesn't use Prolog's
integer division operator, see http://en.wiktionary.org/wiki/faux_ami .)

NO, this should NOT be with a forward slash.  It should be

sum(N) when is_integer(N), N >= 0 ->
    (N * (N + 1)) div 2.
%                 ^^^

> Also the OP should know that this function will return a float.

Not if implemented correctly it won't.

Integer division can be
missing in AWK and Perl and APL (⌊x÷y isn't really the same)
  /     in Fortran, and the C family  (/ is used for fixed division
        in PL/I, but 25+1/3 raises FIXEDOVERFLOW)
  \     MUMPS (officially M these days), also MathWorld
  ÷     in Algol 60 and Algol 68 (where it's also OVER)
  div   in the Pascal family and SML
  `div` in Haskell
  //    in Smalltalk, Prolog, Eiffel, and Python 2.2 and later
  quo:  in Smalltalk (which has 2 of the 5 candidates for what
           "integer division" should mean)
  %/%   in the S family (including R).
  (quotient x y) in Scheme
  (floor x y), (truncate x y) (ceiling x y) (round x y) in Lisp

A programming language I used to like a lot used X / Y to mean
X ++ "/" ++ Y.


_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Richard A. O'Keefe-2
In reply to this post by Roelof Wobben-2

On 11/02/2015, at 8:45 pm, Roelof Wobben <[hidden email]> 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.


_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Roelof Wobben-2
Richard A. O'Keefe schreef op 12-2-2015 om 3:25:

> On 11/02/2015, at 8:45 pm, Roelof Wobben <[hidden email]> 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;
>
>

Thanks for all the remarks. I changed it all but one remark about your
code ;

When I do N1 = N0 + 1 then I get a error because N1 is imutable and
cannot be changed.

So the new code will be :

-module(side_effects).

-export([display_numbers/1]).

%%  display_numbers(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.
display_numbers(Number) when is_integer(Number), Number > 0 ->
   display_numbers_loop(0, Number ).

%%  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) ->
   ok;

%%  When the contro argument(second argument) is not equal to the number
%%  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(Current, Number) ->
   io:format("Number:~p~n",[Current + 1]),
   display_numbers_loop(Current +1, Number ).

Roelof

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Richard A. O'Keefe-2

On 12/02/2015, at 8:03 pm, Roelof Wobben <[hidden email]> wrote:
>
> Thanks for all the remarks. I changed it all but one remark about your code ;
>
> When I do N1 = N0 + 1 then I get a error because N1 is imutable and cannot be changed.

Here's the code I wrote, pasted from that message.

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;

At the point that N1 = N0 + 1 happens,
N1 DOES NOT YET HAVE A VALUE, so there is no possible question of trying to change it.
In fact the code as posted works flawlessly.  (I have tested it.)

I strongly suspect that you ended up with something like

    N0 = N0 + 1
     ^
which will indeed produce the error you describe.



_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: is this a well written function

Richard A. O'Keefe-2
In reply to this post by Roelof Wobben-2
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.

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions