Idiomatic Erlang help

Question

I've got an Erlang snippet here that I'd like to work into more idiomatic Erlang, rather than a crude Python translation.

Process takes a pairs of congruent lists and combines them. Some of the elements need to be taken from one lists or the other, based on their properties, while the rest of the elements need to be summed. It works properly, but I get the feeling it's not idiomatic...

Process = fun([RockA, FishA, TreeA, BarkA, DogA, CowA, MooA, MilkA, CheeseA, BreadA, WineA, GrapesA], [RockB, FishB, TreeB, BarkB, DogB, CowB, MooB, MilkB, CheeseB, BreadB, WineB, GrapesB]) ->
                  if
                      RockA /= [0,0,0] ->
                          NewRock = RockA,
                          NewFish = FishA,
                          NewTree = TreeA,
                          NewBark = BarkA,
                          NewDog = DogA;
                      true ->
                          NewRock = RockB,
                          NewFish = FishB,
                          NewTree = TreeB,
                          NewBark = BarkB,
                          NewDog = DogB
                  end,
                  if
                      CowA > CowB ->
                          NewCow = CowA;
                      true ->
                          NewCow = CowB
                  end,
                  NewMoo = MooA + MooB,
                  NewMilk = MilkA + MilkB,
                  NewCheese = CheeseA + CheeseB,
                  NewBread = BreadA + BreadB,
                  NewWine = WineA + WineB,
                  NewGrapes = GrapesA + GrapesB,
                  [NewRock, NewFish, NewTree, NewBark, NewDog, NewMoo, NewMilk, NewCheese, NewBread, NewWine, NewGrapes];
             (_,_) ->
                  ok
          end.

Answer 1

Here are some suggestions. But it's a matter of taste whether you like the intermediate variable assignments. Just be aware the 'case' and 'if' are expressions that always evaluate to something. I've also removed the "(,) -> ok" catch all; this appears to be defensive programming which is discouraged in Erlang.

Process = fun([RockA, FishA, TreeA, BarkA, DogA, CowA, MooA, MilkA, CheeseA, BreadA, WineA, GrapesA], 
              [RockB, FishB, TreeB, BarkB, DogB, CowB, MooB, MilkB, CheeseB, BreadB, WineB, GrapesB]) ->

            FirstStuff = case RockA of 
                           [ 0,0,0] ->
                             [RockB, FishB, TreeB, BarkB, DogB];
                           _ ->
                             [RockA, FishA, TreeA, BarkA, DogA]
                         end,

              NewCow = if
                         CowA > CowB -> 
                            CowA;
                         true -> 
                            CowB
                       end,

              lists:flatten( [ FirstStuff,
                               NewCow,
                               MooA + MooB,
                               MilkA + MilkB,
                               CheeseA + CheeseB,
                               BreadA + BreadB,
                               WineA + WineB,
                               GrapesA + GrapesB ]);
          end.

Or even...

Process = fun([RockA, FishA, TreeA, BarkA, DogA, CowA, MooA, MilkA, CheeseA, BreadA, WineA, GrapesA], 
              [RockB, FishB, TreeB, BarkB, DogB, CowB, MooB, MilkB, CheeseB, BreadB, WineB, GrapesB]) ->

              lists:flatten( [ case RockA of 
                                 [ 0,0,0] ->
                                   [RockB, FishB, TreeB, BarkB, DogB];
                                 _ ->
                                   [RockA, FishA, TreeA, BarkA, DogA]
                               end,
                               lists:max([CowA,CowB]),
                               MooA + MooB,
                               MilkA + MilkB,
                               CheeseA + CheeseB,
                               BreadA + BreadB,
                               WineA + WineB,
                               GrapesA + GrapesB ]);
          end.

Answer 2

Likely not the most efficient version due to the zipping and appending of lists, but assuming they don't get much longer, it shouldn't be noticeable in most programs compared to the gain in readability.

Process = fun([RockA, FishA, TreeA, BarkA, DogA, CowA, | RestA], 
              [RockB, FishB, TreeB, BarkB, DogB, CowB, | RestB]) ->
              Start = if RockA =:= [0,0,0] ->
                            [RockB, FishB, TreeB, BarkB, DogB];
                         true ->
                            [RockA, FishA, TreeA, BarkA, DogA]
                      end,
              Start ++ [max(CowA, CowB)] ++ [X+Y || {X,Y} <- lists:zip(RestA, RestB)]
          end.

Also notice the lack of catch-all clause when the function call doesn't match. Do not write defensive code. Let it crash and have a supervisor take care of it. Writing defensive code only makes debugging harder in a language where a crash isn't a death sentence on your program.

Answer 3

Yet another solution:

process([RockA, FishA, TreeA, BarkA, DogA | TlA],
        [RockB, FishB, TreeB, BarkB, DogB | TlB]) ->
  case RockA of
    [0,0,0] -> [RockB, FishB, TreeB, BarkB, DogB | process2(TlA, TlB)];
    _       -> [RockA, FishA, TreeA, BarkA, DogA | process2(TlA, TlB)]
  end.

process2([CowA | TlA], [CowB | TlB]) ->
  [erlang:max(CowA, CowB) | process3(TlA, TlB)].

process3([HdA | TlA], [HdB | TlB]) ->
  [HdA + HdB | process3(TlA, TlB)];

process3([], []) -> [].


Process = fun process/2.

Answer 4

Or, taking Zed's answer and replacing the case with function clauses we could do the following. Have we found the idiomatic one yet? Much of it is a matter of taste and esthetic.

process([[0,0,0], _, _, _, _ | TlA],
        [RockB, FishB, TreeB, BarkB, DogB | TlB]) ->
    [RockB, FishB, TreeB, BarkB, DogB | process2(TlA, TlB)];

process([RockA, FishA, TreeA, BarkA, DogA | TlA],
        [_, _, _, _, _ | TlB]) ->
    [RockA, FishA, TreeA, BarkA, DogA | process2(TlA, TlB)].

process2([CowA | TlA], [CowB | TlB]) ->
  [erlang:max(CowA, CowB | process3(TlA, TlB)].

process3([HdA | TlA], [HdB | TlB]) ->
  [HdA + HdB | process3(TlA, TlB)];

process3([], []) -> [].


Process = fun process/2.