[erlang-questions] Refactoring in anger

[Jesper Louis Andersen]

On Sun, Aug 9, 2015 at 12:07 AM, Garrett Smith <g@REDACTED> wrote:

> The original calculate/3 mixes user facing behavior (print error
> messages) with a calculation. If your function returns a mishmash of
> completely unrelated values (e.g. the result of io:format and also the
> result of an area calculation) *it's a mess*.
>

Another point to make could be the area/0 function:

area() ->
  Answer = io:get_line("R)ectangle, T)riangle, or E)llipse > "),
  Shape = char_to_shape(hd(Answer)),
  case Shape of
    rectangle -> Numbers = get_dimensions("width", "height");
    triangle -> Numbers = get_dimensions("base", "height");
    ellipse -> Numbers = get_dimensions("major axis", "minor axis");
    unknown -> Numbers = {error, "Unknown shape " ++ [hd(Answer)]}
  end,

  Area = calculate(Shape, element(1, Numbers), element(2, Numbers)),
  Area.

First, projecting out of Answer with hd/1 can be solved by just pattern
matching the first character out of the list. Of course, an empty list
would fail here, but input parsing is hard and Barbie wants to go shopping.
Second, you don't need to project out of numbers and bind it in every
variant if you use the return value of the case expression to match the
dimensions. Third, in addition to separating side-effects from computation,
it is often smart to let errors return structural data which can be handled
by a machine easily. And then provide a function which can interpret the
machine-readable structure into human-readable form. Garrett omits the
input that were failing. I like to keep it around in the error term[0].
Finally, it should be obvious the answer we wish to return is the area:

area() ->
  [A | _] = io:get_line("R)ectangle, T)riangle, or E)llipse > "),
  {X, Y} = case char_to_shape(A) of
    rectangle -> get_dimensions("width", "height");
    triangle -> get_dimensions("base", "height");
    ellipse -> get_dimensions("major axis", "minor axis");
    unknown -> exit({unknown, A})
  end,

  calculate(Shape, X, Y).

Lifting the case-expr to the top level as Garrett is doing is probably next
up on the todo list. In general, Erlang likes to match data in structure
rather than projecting components out of structure. I'd suggest you master
both styles eventually in code, but matching can often avoid the scourge of
boolean blindness in code.

[0] Caveat: think about when it is being garbage collected and how large
that term is.

-- 
J.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-questions/attachments/20150809/2242ed57/attachment.htm>