novice question - should one use ifs or short functions when programming erlang?

Question

I started studying Programming Erlang book in earnest recently, and I have a question. Is the below proper approach to Erlang? This is (modified for brevity (no quit message), with logging removed after basic verification) solution to the ring problem from ch4. Processes exit after they have passed the message intended number of times; first process waits for the last message to reach it and exits.

Aside from general criticism of style and substance, can you please tell me if writing special 1-2 line functions like this a correct style, or if one should use if-s, cases-s, etc?

start_ring( 0, _, _ ) -> {error, badarg};
start_ring( _, 0, _ ) -> {error, badarg};
start_ring( M, N, Message ) -> 
    spawn( ring, run_ring, [M, N, Message, 0] ).

% last process that connects the ring 
run_ring( M, 1, Message, Pid ) when is_pid(Pid) -> 
    loop_ring( M, Message, Pid, false );

% process in the middle
run_ring( M, N, Message, Pid ) when is_pid(Pid) ->
    loop_ring( M, Message, spawn( ring, run_ring, [M, N-1, Message, Pid] ), false );

% first process - special case for one process
run_ring( M, 1, Message, _ ) -> 
    loop_ring( M, self() ! Message, self(), true );

% first process 
run_ring( M, N, Message, _ ) -> 
    NextPid = spawn( ring, run_ring, [M, N-1, Message, self()] ),
    loop_ring( M, NextPid ! Message, NextPid, true ).

loop_ring( 0, _, _, _ ) -> ok;

loop_ring( 1, Message, Next, true ) -> ok;

loop_ring( M, Message, Next, IsMaster ) ->
    receive 
        Message -> loop_ring( M - 1, Next ! Message, Next, IsMaster )
    end.

Answer 1

I think your style is very good and concise! Good work!

A few comments (a matter of personal taste):

• Start ring can be rewritten as:

  start_ring( M, N, Message ) when M < N, N > 0, M > 0 -> 
     spawn( ring, run_ring, [M, N, Message, 0] ).
  

  This will crash with a function_clause error if used improperly. There's a good habit when dealing with error returns, that if the user can do something sensible with the error, return for example {error, Reason}, otherwise just crash. I think in this case it is safe to just crash, because any other input would be a bug in the program.

• run_ring/4 + loop_ring/4: I don't like to use line breaks between functions with several clauses. That makes it harder to see where the function begins and ends. The comments can then be put inside the clause body instead of outside. It becomes much easier to identify the function headers now (and to see the function as one unit):

  run_ring(M, 1, Message, Pid) when is_pid(Pid) ->
      % last process that connects the ring
      loop_ring(M, Message, Pid, false);
  run_ring(M, N, Message, Pid) when is_pid(Pid) ->
      % process in the middle
      loop_ring(M, Message, spawn(ring, run_ring, [M, N-1, Message, Pid]), false);
  run_ring(M, 1, Message, _) ->
      % first process - special case for one process
      loop_ring(M, self() ! Message, self(), true);
  run_ring(M, N, Message, _) ->
      % first process
      NextPid = spawn(ring, run_ring, [M, N-1, Message, self()]),
      loop_ring(M, NextPid ! Message, NextPid, true).
  

• I personally dislike spaces inside parentheses (as a I said, personal taste). :-) Makes the code more "fluffy".

• Use spawn_link/3 instead of spawn/3 unless you know that you don't want it. It makes it much easier to detect bugs etc when you're developing your program.

• The second clause of loop_ring/4 emits compiler warnings. Use _Message and _Next instead (use them for the first clause as well, it's bonus documentation!)

Answer 2

According to the Erlang best practices, one should avoid if and case nested more than twice:

Nested code is code containing case/if/receive statements within other case/if/receive statements. It is bad programming style to write deeply nested code - the code has a tendency to drift across the page to the right and soon becomes unreadable. Try to limit most of your code to a maximum of two levels of indentation. This can be achieved by dividing the code into shorter functions.

Apart from that, I guess it is a matter of taste to use an if/case or simple pattern matching. Personally, I prefer using pattern matching rather than if or cases. So, you're doing it the right way if you ask me.

Regarding if and case, usually you can rewrite the former into the latter. Someone says:

"always use the case, the if construct is kinda broken".

Well, the two constructs work very differently. The expression evaluated in the if construct is a guard and it has a lot of limits - due to the fact you can't have side effects into guards, being evaluated irrespectively of the branch taken -. The case construct doesn't have this "limitation". You can use any expression there, whose result will be matched against the patterns forming the case.

Possible duplicate:

Erlang style - case vs function pattern matching