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