Overuse of guards in Erlang?

Question

Hi,

I have the following function that takes a number like 5 and creates a list of all the numbers from 1 to that number so create(5). returns [1,2,3,4,5].

I have over used guards I think and was wondering if there is a better way to write the following:

create(N) ->
    create(1, N).

create(N,M) when N =:= M ->
    [N];
create(N,M) when N < M ->
    [N] ++ create(N + 1, M).

Thanks,

Paul

Answer 1

create(N,N) -> [N];
create(N,M) -> [N|create(N + 1, M)]. % Don't use ++ to prefix a single element.

This isn't quite the same (you could supply -5), but it behaves the same if you supply meaningful inputs. I wouldn't bother with the extra check anyway, since the process will crash very quickly either way.

BTW, you have a recursion depth problem with the code as-is. This will fix it:

create(N) ->
    create(1, N, []).

create(N, N, Acc) -> [N|Acc];
create(N, M, Acc) -> create(N, M - 1, [M|Acc]).

Answer 2

The guard for N < M can be useful. In general, you don't need a guard for equality; you can use pattern-matching.

create(N) -> create(1, N).

create(M, M) -> [M];
create(N, M) when N < M -> [N | create(N + 1, M)].

You also generally want to write functions so they are tail-recursive, in which the general idiom is to write to the head and then reverse at the end.

create(N) -> create(1, N, []).

create(M, M, Acc) -> lists:reverse([M | Acc]);
create(N, M, Acc) when N < M -> create(N + 1, M, [N | Acc]).

(Of course, with this specific example, you can alternatively build the results in the reverse order going down to 1 instead of up to M, which would make the lists:reverse call unnecessary.)

If create/2 (or create/3) is not exported and you put an appropriate guard on create/1, the extra N < M guard might be overkill. I generally only check on the exported functions and trust my own internal functions.

Answer 3

I don't really think you have over used guards. There are two cases:

The first is the explicit equality test in the first clause of create/2

create(N, M) when N =:= M -> [M];

Some have suggested transforming this to use pattern matching like

create(N, N) -> [N];

In this case it makes no difference as the compiler internally transforms the pattern matching version to what you have written. You can safely pick which version you think feels best in each case.

In the second case you need some form of sanity check that the value of the argument in the range you expect it to be. Doing in every loop is unnecessary and I would move it to an equivalent test in create/1:

create(M) when M > 1 -> create(1, M).

If you want to use an accumulator I would personally use the count version as it saves reversing the list at the end. If the list is not long I think the difference is very small and you can pick the version which feels most clear to you. Anyway, it is very easy to change later if you find it to be critical.