[erlang-questions] Refactoring in anger

[Garrett Smith]

On Sun, Aug 9, 2015 at 3:48 AM, Jesper Louis Andersen
<jesper.louis.andersen@REDACTED> wrote:
>
> 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].

Just to elaborate on Jesper's point, this function:

  char_to_shape("R") -> rectangle;
  ...
  char_to_shape(_) -> error(bad_shape).

would become

  char_to_shape("R") -> rectangle;
  ...
  char_to_shape(Other) -> error({bad_shape, Other}).

This is generally a good idea and I initially had this and used the
invalid value in a message to the user. But as the UI is an interview
that errors out on the first invalid value, printing the bad value was
a bit redundant so in the in interest of simplifying the example I
yanked it.

But Jesper's right - it's a better general practice to include the
offending value as context in an exception. In fact this is a major
use of exceptions - to capture context of an error that would
otherwise not be available in a bad match, case clause, etc.

Garrett