[erlang-questions] Refactoring in anger

[Garrett Smith]

Aye, this was an untested change I made after posting the original.
Change reverted.

On Sat, Aug 8, 2015 at 8:19 PM, Leandro Ostera <leandro@REDACTED> wrote:
> Just a minor typo: Line 48 should is trying to reassign the Prompt variable.
>
> But +1 on the rest.
>
> On Sun, Aug 9, 2015 at 2:36 AM, Felix Gallo <felixgallo@REDACTED> wrote:
>>
>> Eshell V7.0  (abort with ^G)
>> 1> c(geom).
>> {ok,geom}
>> 2> c(ask_area).
>> {ok,ask_area}
>> 3> ask_area:print_area().
>> R)ectangle, T)riangle, or E)llipse > T
>> ** exception error: no function clause matching
>> ask_area:error_msg({badmatch,["Enter ","base"," > "]}) (ask_area.erl, line
>> 69)
>>      in function  ask_area:print_area/0 (ask_area.erl, line 9)
>>
>>
>> glass houses, etc.
>>
>> F.
>>
>> On Sat, Aug 8, 2015 at 3:07 PM, Garrett Smith <g@REDACTED> wrote:
>>>
>>> We've had an ongoing thread on how to handle some problems
>>> idiomatically in Erlang:
>>>
>>> http://erlang.org/pipermail/erlang-questions/2015-August/085382.html
>>>
>>> The OP's questions are coming out of an exercise here:
>>>
>>> http://chimera.labs.oreilly.com/books/1234000000726/ch05.html#CH05-ET01
>>>
>>> This in turn points to a proposed solution here:
>>>
>>>
>>> http://chimera.labs.oreilly.com/books/1234000000726/apa.html#_literal_geom_erl_literal_9
>>>
>>> Upon reading this solution, I became so enraged [1] that I rewrote the
>>> module and want to make a number of points.
>>>
>>> Here's my rewrite:
>>>
>>> https://gist.github.com/gar1t/7bb80d728f804554ac32
>>>
>>> The tone of my points below is *very preachy* which is going to annoy
>>> some people here. I apologize in advance - but hey, ranters gotta
>>> rant.
>>>
>>> # Clarity of "what's going on"
>>>
>>> Compare my area/0 to the original area/0. Which is easier to see
>>> "what's going on"? I'm not boasting here but rather making the most
>>> important point I can about programming: take the time to be clear in
>>> your intent! If you're actually clear - in your brain - making the
>>> code reflect your understanding *is not that hard*. If your code is
>>> not clear, chances are your brain is not clear.
>>>
>>> Maybe the original code works, maybe it doesn't. I can't tell from
>>> looking at that function, at all. I have to dig around various
>>> implementation details and hold a bunch of information in my brain to
>>> understand what the author is maybe trying to do. At some point I
>>> can't keep it all straight and have to run the thing and observe its
>>> behavior. *Now* I'm thinking, what happens to the pour schlep who has
>>> to modify this code. In the real world, this causes fear, and
>>> loathing, and tests - lots and lots of tests!
>>>
>>> We don't have to live this way.
>>>
>>> # Separation of concerns (in this case IO vs calculations)
>>>
>>> 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*.
>>>
>>> In the rewrite I've added print_area/0, which is responsible for
>>> displaying results to the user. area/0 returns an area or raises an
>>> exception. print_area/0 handles both the result and any error.
>>>
>>> # Handling invalid input
>>>
>>> The original thread here involves a discussion on how to handle bad
>>> input to a function. My rewrite does one of two things on this front:
>>>
>>> - If input is from a user, there's an explicit exception raised on bad
>>> input that can be used by an error handler to inform the user
>>>
>>> - If input is not from a user but rather internal, I don't handle bad
>>> input at all, but let Erlang crash with a function or case clause
>>> error
>>>
>>> The first case address the user experience. We could let exceptions
>>> just propagate and upset users with arcane Erlang messages. Or we can
>>> handle errors politely with intelligible messages.
>>>
>>> The original ask_erl.erl handles invalid input by passing atoms like
>>> 'error' and 'unknown' along a call chain. This is tedious and finally
>>> culminates in the original calculate/3 - a monster mishmash function
>>> of error handling, IO, and calculation.
>>>
>>> My rewrite raises an exception for those functions that take user
>>> provided input. I prefer exceptions in this case as they keep return
>>> values on the happy path, which makes code easier to read.
>>>
>>> I don't care about handling internal errors, as long as they manifest
>>> in an obvious way.
>>>
>>> # Avoid variable assignment/binding inside case expressions
>>>
>>> Just don't do this:
>>>
>>>   case Shape of
>>>       rectangle -> Numbers = get_dimensions("width", "height");
>>>       triangle -> Numbers = get_dimensions("base", "height");
>>>       ellipse -> Numbers = get_dimensions("major axis", "minor axis")
>>>   end
>>>
>>> Now that you're not allowed to do that, what? Hey, a function!
>>>
>>>   Numbers = get_dimensions(Shape)
>>>
>>> Every time, all the time.
>>>
>>> # Consider not using case expressions at all
>>>
>>> Think of a function as a named case expression with a well defined scope.
>>>
>>> You'll find that having to name the function forces you to think about
>>> "what's going on" there. It will help your reader (often that means
>>> you, later on) to understand your intention.
>>>
>>> My rewrite doesn't use a single case expression. Is it somehow worse?
>>> It's better!
>>>
>>> # If it looks confusing, it's bad!
>>>
>>> Erlang is not C, or bash, or Perl, or Ruby. It's possible to write
>>> really easy-to-read code in Erlang. People who complain about Erlang
>>> syntax are probably complain about terrible code in Erlang. Terrible
>>> code in any language is worth complaining about - but it's unrelated
>>> to syntax.
>>>
>>> It's easy to spot bad code in Erlang:
>>>
>>> - Long functions
>>>
>>> - Excessive nesting (more than two levels is a train wreck, and IMO
>>> more than one is bad)
>>>
>>> I hate nesting so much that I'll go to the trouble of writing
>>> to_number/1 as a list of "try" attempts (see rewrite). Some people
>>> call this monadic, which I like because it sounds super cool.
>>>
>>> - Variable assignment/binding inside case and if expressions (see above)
>>>
>>> - Functions that are a litany of imperative style instructions, like
>>> this:
>>>
>>>   get_number(Prompt) ->
>>>       Str = io:get_line("Enter " ++ Prompt ++ " > "),
>>>       {Test, _} = string:to_float(Str),
>>>       case Test of
>>>           error -> {N, _} = string:to_integer(Str);
>>>           _ -> N = Test
>>>       end,
>>>       N.
>>>
>>> This fails the "long functions" test - but lines per function is just
>>> a proxy. The real problem here is that it takes too much effort to
>>> read and understand. Really, this is an imperative pattern applied
>>> naively to Erlang. No!
>>>
>>> Try this:
>>>
>>>   number_from_user(Prompt) ->
>>>       to_positive_number(prompt_user(["Enter ", Prompt, " > "])).
>>>
>>> Where's the rest of the code? It's there, inside the other functions.
>>> But *this* function doesn't make you deal with that detail because it
>>> wants you to understand what *it* is doing.
>>>
>>> Okay, so I've been pretty critical here of this online training
>>> resource, which some will consider bad form. So let's turn this into
>>> something nice and kind!
>>>
>>> The subject of this email is "refactoring in anger", which turns out
>>> to be something I routinely do with my own code. It's hard to write
>>> the perfect code in one pass. So I tend to just "get it to work" and
>>> then examine what seems to be working very carefully - and then change
>>> it so that it becomes *super obvious* what's going on. This state can
>>> take a few passes and still might have warts. Okay, no sweat - it's
>>> just code. Nothing to get angry about. Calm down and fix it.
>>>
>>> Over time, when you practice this pattern of refactoring, you'll start
>>> writing things clearly the first time. That's because you'll start
>>> *thinking* more clearly the first time.
>>>
>>> Now that's positive and nice and kind right?
>>>
>>> Garrett
>>>
>>> ---
>>>
>>> [1] Of course I'm joking here, but only partly. The original solution
>>> is presented to learners - and it's full of bad practices, so that
>>> makes me cranky. I suppose it's good to have teaching material for
>>> Erlang in any form - at least it serves as a point of discussion, even
>>> if contentious.
>>> _______________________________________________
>>> erlang-questions mailing list
>>> erlang-questions@REDACTED
>>> http://erlang.org/mailman/listinfo/erlang-questions
>>
>>
>>
>> _______________________________________________
>> erlang-questions mailing list
>> erlang-questions@REDACTED
>> http://erlang.org/mailman/listinfo/erlang-questions
>>
>