[erlang-questions] Code critique please

[Bernard Duggan]

Jarrod Roberson wrote:
>           case Response#dns_rr.ttl == 0 of
>               true ->
>                   NewSub = dict:store(SD,dict:new(),Sub),
>                   process_dnsrec1(NewSub,[]);
>               false ->
>                   % update the dns_rr to the current timestamp
>                   NewRR = Response#dns_rr{tm=get_timestamp()},
>                   NewValue = dict:store(Key,NewRR,Value),
>                   NewSub = dict:store(SD,NewValue,Sub),
>                   process_dnsrec1(NewSub,Rest)
>           end;
>
>   
Traditionally the inner case block would be done as:

Case Response#dns_rr.ttl of
    0 -> % do stuff that was in 'true' block
    _ -> % do stuff that was in 'false' block
end;

Further, if you do do a comparison against 0, nine times out of ten you
will want to use =:= rather than ==.  The former is an exact match
against the integer 0; the latter will also match the floating point
value 0.0.  It may not seem important now, but if, to take one example,
you start using mnesia and qlc it's a really good habit to be in.

If the two-level indenting/nesting of case statements bothers you, the
easiest way is just to break the inner case statement out into another
function..or even a function with two definitions and do away with the
inner 'case' entirely:

process_ttl(0) -> % do stuff in 'true' block;
process_ttl(_) -> % do stuff in 'false' block

Also, from a stylistic point of view, I'd be inclined to alter the
return value of is_subscribed from {ok, S} to {true, S}, just because
the function name appears to ask a question, and "ok" isn't really an
answer :)

Of course, that's all just my opinion - I'm still learning this stuff
too (aren't we all?) ;)

Cheers,

Bernard