[erlang-questions] Refactoring in anger (Felix Gallo)
Roelof Wobben
r.wobben@REDACTED
Sun Aug 9 10:39:56 CEST 2015
Op 9-8-2015 om 10:31 schreef Roelof Wobben:
> Op 9-8-2015 om 8:25 schreef J David Eisenberg:
>> Felix Gallo wrote:
>>
>>> Nevertheless, if you're going to level a thundering, public j'accuse at
>>> someone who has clearly gone to great effort to provide a beginner
>>> experience for erlang newbies, without (apparently) first contacting
>>> him
>>> privately and suggesting improvements in a constructively critical
>>> manner,
>>> please have the decency to run the code that you have etched into your
>>> stone tablets *before* you hold them aloft and, with fiery mane
>>> ablaze in
>>> the evening sun, present them as the replacement for the gentleman's
>>> work.
>>>
>>> There are many "problems" with the etudes code. I'm not sure that
>>> introducing a complex exception handling workflow is one of them, to be
>>> frank, pedagogically, at the moment that the student is trying to
>>> understand pattern matching and function headers. Could be. But I
>>> think
>>> we can say it's a matter of taste.
>>>
>>> And that's what really rubs me the wrong way about Garrett's post.
>>> Being
>>> 'very preachy', even when volubly disclaimed, is still pretty
>>> tasteless.
>>> But being 'very preachy' and then slapping up code you haven't even run
>>> once, directly in opposition to your preaching's core point: that's
>>> hypocritical, and super tasteless. And I enjoy my rich dark ironic
>>> comedy
>>> as much as the next guy, but come on.
>>>
>>> F.
>> Author of Etudes here.
>>
>> There I was at Pepe's Tacos in Las Vegas, waiting for my meal, and I
>> decided
>> to read the Erlang questions mail list and see what's going on. As
>> the story
>> unfolded, I started cackling with laughter, until I realized that half
>> the people
>> in the restaurant were staring at me like "What the hell is wrong
>> with this
>> crazy white guy laughing at his phone?"
>>
>> So no, I'm not even remotely offended. Especially because I write in
>> the preface:
>> "I was learning Erlang as I was creating the solutions to the études,
>> following the philosophy that 'the first way that works is the right
>> way.'
>> Therefore, don’t be surprised if you see some fairly naïve code that
>> an expert
>> Erlang programmer would never write."
>>
>> Since the book is open source, if anyone would like to add a better
>> solution with
>> a discussion of why it's better, I would be perfectly happy to see
>> that. Modulo
>> the anger, of course :)
>>
>>> On Sat, Aug 8, 2015 at 6: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
>
> I was the person who was looking for help because I was more then 1
> week busy with this exercise.
> I understand the solution presented by Garret. The only things I miss
> are the documentation of the functions. (spec and explanations).
>
> Roelof
>
>
>
> ---
> Dit e-mailbericht is gecontroleerd op virussen met Avast
> antivirussoftware.
> https://www.avast.com/antivirus
>
> _______________________________________________
> erlang-questions mailing list
> erlang-questions@REDACTED
> http://erlang.org/mailman/listinfo/erlang-questions
I found two problems.
one that area/0 is not found. That could be easily solved by adding
area/0 to export.
Second one that area/3 cannot be found. Somehow that one is not imported.
Roelof
---
Dit e-mailbericht is gecontroleerd op virussen met Avast antivirussoftware.
https://www.avast.com/antivirus
More information about the erlang-questions
mailing list