[erlang-questions] Re: The If expression

[James Aimonetti]

This thread is well-timed as I have an issue I wanted some help on.

I have two lists, one of keys and one of values. I want to zip them 
together, but I have one specific key/value combo that I want to exclude 
from the zipped list. For context, this is creating an appropriate 
{proplist} for couchbeam.

to_couchbeam(Fields, Values) ->
  { lists:flatten( lists:zipwith(fun to_couchbeam_zipper/2, Fields, 
Values) ) }.

to_couchbeam_zipper(F, V) ->
  K = to_couchbeam_key(F),
  Val = to_couchbeam_value(V),
  case Key == <<"_rev">> andalso Val == undefined of
    true -> [];
    false -> {K, Val}
  end.

to_couchbeam_* converts various terms to formats suitable for couchbeam 
to save the document. I only want to remove the '_rev' key when it's 
value is undefined, otherwise couchbeam fails to save the document. If 
'_rev' has a defined value, great, keep it in the generated proplist.

I refactored according to some of the advice earlier, so now 
to_couchbeam_zipper is

to_couchbeam_zipper('_rev', undefined) -> [];
to_couchbeam_zipper(F, V) -> { to_couchbeam_key(F), to_couchbeam_value(V) }.

So I may have answered my own question here, as concerns the case 
statement. Is there a cleaner way to zip two lists in this fashion 
without resorting to inserting an empty list for unwanted k/v pairs and 
flattening the resulting proplist?

Thanks,

James

Jay Nelson wrote:
> Henning Deidrich offered one-armed ifs to rewrite:
>
> I never use the if statement.  It doesn't even occur to me to use it, 
> although writing Java or something else I use it all the time 
> naturally.  Here's the style I would use for some of your cases (some 
> of the others may not have enough context to be good examples for 
> rewriting):
>
> > (2)
> >        verbose_echo(Verbose, Format, Para) ->
> >
> >            % Don't print if verbosity is off (false).
> >         >>> if Verbose -> echo(Format, Para); true -> nil end.
>
> verbose_echo(true, Format, Para) ->
>     echo(Format, Para);
> verbose_echo(_, _Format, _Para) ->
>     nil.
>
> It doesn't do what the comment says, and you have to consider whether 
> nil is the value you want back.  It depends on what echo/2 returns as 
> to what is appropriate (and whether the caller even cares about the 
> return value).  With this approach it is easier to test and easier to 
> add new cases (verbosity levels) or change the existing one.
>
>
> >    (3)
> >            % If exists, drop old before adding new element
> >            Prev = gl:get(suite, Suite),
> >         >>>    if Prev /= undefined -> gl:drop(suite, Suite); true ->
> >        nil end,
> >            gl:add(suite, Suite),
>
> Prev = case gl:get(suite, Suite) ->
>     undefined -> nil;
>     Defined -> gl:drop(suite, Suite), Defined
> end,
> gl:add(suite, Suite),
>
>
> Here I made the assumption that you needed the value of Prev later.  
> If you don't, you can drop it out and not care what the return value 
> of the case statement is.
>
> This is actually a common code smell to watch for.  You are setting a 
> variable, then testing for a negative value using /= constant.  
> Instead, use case and make the first clause the constant you want to 
> exclude, everything else will match the catch-all clause.
>
> Even when I have case ... of true -> ...; false -> ... end   I find it 
> more comforting in erlang to know I covered the cases I care about.  
> You can even use a single armed case if you want a freak value to 
> crash your process.
>
> > (5)
> >      safe_unregister(Name) ->
> >      Registered = whereis(Name) /= undefined,
> >       >>> if Registered -> unregister(Name); true -> nil end.
>
> safe_unregister(Name) ->
>   case whereis(Name) of
>     undefined -> ok;
>     Other -> unregister(Name), ok
>   end.
>
> Another instance of #3.  You can easily fix the return values to 
> different ones, or have a return value after the case ... end.  If you 
> need to know the value of whereis, you can modify it to this:
>
> safe_unregister(Name) ->
>    Loc = case whereis(Name) of
>        undefined -> undefined;
>        Other -> unregister(Name), Other
>    end,
>    ... computations involving Loc ...
>   %% or even return it...
>   Loc.
>
>
> Any time you find yourself binding a variable, and then soon after 
> wanting to modify it or do an if based on a small set of values, stop, 
> back up and try using a case statement.  It has the advantage of 
> giving you multiple variables with delayed binding of the correct one 
> as your chosen value:
>
> Chosen = case multi_var_fun() of
>           Val1 -> Val1;
>           Val2 -> f(Val2);
>           Val3 when Val3 > 0, Val3 < 5 -> g(Val3);
>           Other -> h(Other)
> end,
>
> Here Chosen is a late-bound variable with several values visited as 
> intermediate bindings before arriving at a final choice.  An 
> imperative programmer tries to do something like:
>
> Chosen = multi_var_fun(),
> if (Chosen == Val1) ...
> else ...
>
> The erlang way is to avoid binding the variable you care about until 
> you are absolutely sure you have the _value_ you care about, rather 
> than modifying the value along the way.   A one-armed if is 
> semantically like saying I want Val1 most of the time, and in a few 
> cases I want to change it to a different value.
>
> jay
>
>
> ________________________________________________________________
> erlang-questions (at) erlang.org mailing list.
> See http://www.erlang.org/faq.html
> To unsubscribe; mailto:erlang-questions-unsubscribe@REDACTED
>
>


-- 
James Aimonetti
mobile: 314.809.6307
work: 540.459.2220
email: james.aimonetti@REDACTED
website: http://jamesaimonetti.com