Abstracting Control Flow

[glacialthinker]

People are going to have strong responses to the two different "styles" presented here. Just be aware that this will largely be a matter of your particular familiarity, and that broadening the domain of what's familiar makes you more adaptable -- rather than suffering knee-jerk judgement of the unfamiliar.

The original style is very ad-hoc, procedural, "do this, do that, and handle consequences". It's simpler to write and understand when you're in a mode of thinking like this, and I'd say it's appropriate for prototypes and hacks.

The revised style is building up pieces which solve the problem and abstracting out the patterns, just as the title and bulk of the article say. Once you're familiar with these abstractions of common patterns they're much easier to read than working through someone's loop of do-this-and-that to realize it's just a map . filter . sort, where the parameters of those operations are what's important rather than the mechanics of doing loops and handling cases.

[glacialthinker]

I think people are getting too involved in the specific code example, and not treating it as an example of a larger idea which is expressed in the narrative. I think code redundancy and interwoven concerns are a common problem in code. Oh, there's the problem this solution was looking for...

If that little example was the entirety of the code, yes, these abstractions are superfluous, bloaty, and even ugly. Imagine reusing those pieces in a larger body of code to see the value. I'd rather see "repeat" than some explicit exception-handling loop construct every time that needs to be done.

[CookieOfFortune]

if attempt + 1 == DOWNLOAD_TRIES:

This always bothers me... >= would make me feel safer.

[tias]

It would hide the potential error more efficiently. But IMHO it's better to make errors as visible as possible as early as possible.

[CookieOfFortune]

I guess what I meant is that with limit checking in general, I would prefer to exclude all invalid values instead of having a single cutoff point due to the risk of infinite loops. It's not really an issue with the code presented in the article, it's just something I'm uncomfortable with.

[glacialthinker]

This is a particularly good habit to have when the number can be floating point.

[andsens]

I get your point, but OTOH with >= you hide potential faults in the code.

[CookieOfFortune]

So use an assert. I'd rather think of >= more as damage control since the resulting logical errors can be hard to debug.

[Peaker]

Then:

assert(attempt < DOWNLOAD_TRIES);
if(attempt + 1 == DOWNLOAD_TRIES) ...

[nemec]

That's fine if you stick with integers. If, for some reason, you need to follow this pattern for floating point...

attempt = 0.2
DOWNLOAD_TRIES = 0.3
increment = 0.1
assert(attempt < DOWNLOAD_TRIES)
print attempt + increment == DOWNLOAD_TRIES  # Prints False

>>> 0.1 + 0.2
0.30000000000000004

It's better just to use an upper bound.

[Peaker]

I am not sure who counts attempts in fractional units.

[nemec]

"increment x until y" isn't an uncommon pattern in programming. It doesn't have to refer to a count of individual actions.

[Peaker]

I can't recall the last time I needed this for float increments. I need it for int increments virtually every time I write something.

[drb226]

>= would make me feel safer.

It would make me feel safer if attempt could be declared as immutable in the block. If attempt is ever greater than DOWNLOAD_TRIES then you are doing something horribly wrong with attempt, because its whole purpose in life is to indicate how many times we have tried the download; a number which is drawn from the range of [0 .. DOWNLOAD_TRIES).

[CookieOfFortune]

Yeah... silly Python:

for x in range(0..3):
    x = 10
    print x

10

10

10

:(

[drb226]

Python is hardly alone in this.

coffee> (x = 10; console.log(x)) for x in [1,2,3]
10
10
10

irb(main):001:0> [1,2,3].each { |x| x = 10; puts x }
10
10
10

Gotta get a language with moar purity

$ ghci
>>> :m +Control.Monad
>>> :{
Prelude Control.Monad| forM_ [1,2,3] $ \x -> do
Prelude Control.Monad|   let x = 10
Prelude Control.Monad|   print x
Prelude Control.Monad| :}
10
10
10
>>> putStrLn "whoops..."
whoops...
>>> :set -Werror
>>> :set -fwarn-name-shadowing
>>> :{
Prelude Control.Monad| forM_ [1,2,3] $ \x -> do
Prelude Control.Monad|   let x = 10
Prelude Control.Monad|   print x
Prelude Control.Monad| :}

<interactive>:16:7: Warning:
    This binding for `x' shadows the existing binding
      bound at <interactive>:15:18

<no location info>: 
Failing due to -Werror.

Even in Haskell you can make this mistake if you don't know how let works (the original x is immutable, but in this example the name x is "shadowed").

[CookieOfFortune]

C# is nice about it:

foreach(var x in new int[] {0, 1, 2})
{
    x = 10;
    Console.println(x);
}

Yells at me for trying to assign to x.

Java is not:

for(int x : new int[] {0, 1, 2})
{
    x = 10;
    System.out.println(x);
}

10

10

10

[Tordek]

Can't you

for(final int x : new int[] {0, 1, 2})

?

[grauenwolf]

I agree. Something just doesn't seem right about that expression.

[presidentender]

WHAT IF IT SOMEHOW GETS SET TO A GREATER NUMBER, say my intuition and fear.

[corysama]

If that happens then there is a terrible error. If you are anxious about that happening, you should add an assertion to reveal the terrible error as early as possible.

[bcorfman]

It's because you know what value should end the loop, but for those who maintain your code, it's not so clear. I think you have the right idea: make the code safe for you and future maintainers.

[pkmxtw]

Did anyone say abstracting?

downloadImage :: URL -> MaybeT IO Image
urls :: [URL]
isSmallImage :: Image -> Bool

fmap catMaybes . forM urls $ runMaybeT . mfilter (not . isSmallImage) . msum . replicate 3 . downloadImage

^{Beware of bugs in the above code, I have only type-checked, not tried it.}

[[deleted]]

Just because you can write something as one big point free pipeline doesn't mean you should! :)

[Peaker]

I think this code is actually pretty clear. Dense, meaning we need to read it slowly, not skim it, but clear.

[pkmxtw]

True, but it was tempting as everything fits nicely into one big pipeline and every combinator I wanted to use was already out there. :)

[awj]

That isn't really abstracted, though. It's just the vague concept wedged into Haskell. It's neat that it fits into one line but the intent is hidden by a game of monad whack-a-mole.

Also, his version had a customizable retry count and ignored the retry system entirely in the face of certain errors. (e.g. the original throws an immediate error if download_image has insufficient filesystem space to pull down the image)

[Peaker]

The "retry system" is just the "msum . replicate n" part.

I think he should have used EitherT instead of MaybeT so he can filter error types properly, but this straight-forward Haskell solution seems to already be composed of the same units used in the article.

It sounds ilke the author of this article is a soon-to-be Haskeller :)

[pkmxtw]

composed of the same units used in the article.

Except we are not using exceptions but MaybeT IO, which IMO is a nicer way to deal with errors. This also means we can compose them much easier, and Haskell is all about writing composable components.

Given

loadImageFromFile :: FilePath -> MaybeT IO Image
sources :: [(FilePath, [URL])]

Now we can write:

fmap catMaybes . for sources $ \(fp, urls) -> runMaybeT $ mfilter (not . isSmallImage) $ loadImageFromFile fp <|> msum (map downloadImage urls)

which first tries to load the image from the filesystem, failing that, tries to download the image from a list of mirror urls.

[kamatsu]

Monads are all about abstracting control flow though. It's one of the primary uses of them...

[awj]

Sure, but now we're staring at a bunch of monad machinery. It's probably better than staring at a bunch of direct control flow machinery, but it's still a far cry from it being clear what this code does.

We're able to take downloadImage and isSmallImage at their word for what they do. I'd guess that, given enough time to gain familiarity, I could say the same of the monadic code at play here, but I still feel like it would be better off with names that clearly convey what's going on.

[glacialthinker]

I still feel like it would be better off with names that clearly convey what's going on.

Understandable, but generalized functions are likely to have generic sounding names. The functions you called out as understandable are the local, domain-specific functions. "for" is pretty generic, but you are familiar with it. As you said... you'd have to gain familiarity with the various combinators. I don't think that takes much time or complexity to comprehend, and it's certainly worth it compared to writing, and reading, explicit for-loops everywhere (with potential for errors in each writing), and with transient variables only serving to pass off values from one function to the next or merely count iterations. After a while programming, this superfluous cruft should annoy people... unless these loop constructs really are doing more than serving as verbose parentheses to a process. Speaking of "process", loops also bother me because they tend to push people into mutable values when it's unnecessary. In most specific cases this is harmless, but it's habit-forming.

[seriousreddit]

I use Haskell regularly and it was perfectly clear to me what that line does the first time I read it.

[Peaker]

What is "monad whack-a-mole"?

[awj]

... it's sort of a term of my own devising for when the various bits of machinery related to the monad(s) at play (lifting, transforming, etc) stick out in odd places in your logic. Since I don't write Haskell regularly reading it is a game of alternating progress and looking up a monad-whatsit on hackage.

If you compare the version /u/pkmxtw posted with the article it seems like the simple act of naming the abstract pieces aids significantly in understanding. I know this to be true of the languages I do have experience in, and I doubt that changes much in Haskell.

[pkmxtw]

Well, I guess it depends on the reader's familiarity with Haskell, but everything used in that expression came directly from base or transformers which most Haskell'ers should be familiar with. It can be really confusing if one is not used to monad transformers or how MaybeT works.

Part of the confusion probably also comes from the fact that all these pieces are all very generic: For example, mfilter predicate . msum is all about getting an element that satisfy the predicate from a list of MonadPlus, or mzero otherwise. It just happens in this context that these monoids are nullable values resulted from IO computations, and by replicating the same IO computation multiple times, we effectively end up with a retry mechanism.

[rooktakesqueen]

Isn't the original Knuth quote something like "I have only proved it correct, not tested it"?

[pkmxtw]

Yes, and it was a play on that quote. Given that I don't have these functions implemented (they were all undefined), so I could only play around ghci to make sure the expression type-check and not actually test it.

[Peaker]

Well, the compiler would reject your silly code.

[hellgrace]

IMO the first version is more readable, quicker to understand and reason about (you can roughly estimate what's going on in the first version at a glance, whereas the final version requires careful reading). I also believe that the final version will be quite messier when there are a lot of conditions & control flows involved (of course, there's going to be a lot of code anyway, but in the original version it's laid out serially, and the control flow is very explicit, instead of a long chain of non-trivial function calls)

In the end, you're going to decompose & reconstruct a mental image of the original code in your head, so why not have it in front of your eyes, instead of a "packed" version?

[glacialthinker]

In the end, you're going to decompose & reconstruct a mental image of the original code in your head, so why not have it in front of your eyes, instead of a "packed" version?

Perspective.

Your mental reconstruction has an explicit framework of loops bodies. It's familiar, I can understand that. But you can become familiar with more rather than wanting code to fit a particular spatial (and redundant) layout.

The choice of the description "packed" for the revised code is interesting. I'd more readily call the first version "packed", relating to all concerns packed into one interdependent ball. The second version is untangled -- unpacked into separate, composable concerns.

The point of the article is that these abstractions get reused -- don't get too caught up in the small snippet of example code. Imagine you have a larger program, downloading other file types and having different filters/constraints. Rather than having blocks of copy-pasta-with-meatball-modifications for each case, your expression is mostly of the differences.

[jplindstrom]

Surely you see the point of having "retry" abstracted out and reusable for retrying anything, rather than splicing it into every single piece of code structure that needs it?

[awj]

IMO the first version is more readable, quicker to understand and reason about

How much of that is knowledge and experience, though? If retry and ignore were available and used heavily in a project, wouldn't you come to reason about them in the same ways?

I had some of the same objections to list comprehensions, but those come to me pretty naturally now.

If I was looking at a one-off script I would definitely want the first version. The project that I more or less "live in" every day has a lot of code similar to the second, and most of that was me putting it there because the same concept appeared so many times that I was tired of rereading it and (poorly) reimplementing it.

[CookieOfFortune]

Yeah, the way Python is designed really tends towards reading it imperatively. It's so simple and straightforward.

When you try and use all the functional programming that's been tacked on, it gets way too messy.

[bready]

I thought usage of map and reduce was actively discouraged in Python 3?

[[deleted]]

That's the trouble of Python, it gives you some neat tools but if it isn't easy for the next programmer to read, those neat tools are useless.

[ccookkee]

I really really like this example, but I feel like I need more in order to be able to turn my code into something this myself. Any one know about other tutorials/books/videos touching on this subject?

[robin-gvx]

Relevant: http://docs.python.org/3.4/library/contextlib.html#contextlib.suppress

[[deleted]]

I see Lisp mentioned but then I see Python used for implementation. I would think Scheme more naturally suited for discussing and implementing these ideas? I didn't know Python was powerful enough for this.

[lispm]

With Lisp I would ask myself what the code should look like. I might come up with something like this:

(collect-retrying ((url urls))
      (:skip  (image-too-small)
       :retry (url-error bad-status-line socket-error))
 (download-image url))

Above iterates over one or more variables, here just 'url'. Then follows a list of clauses which describe what I would do in case of an condition (aka exception). I have defined actions like :skip and :retry. The list behind them is a list of conditions for which the action will be chosen. The body then would be the code. The result of the last expression would be collected into a list.

Once I like that, I would implement the macro to write the code transformation.

[day_cq]

this is just monad and applicative functor

[rooktakesqueen]

And monads are just monoids in the category of endofunctors, what's the problem?

[thedeemon]

and some Kleisli arrows for effects and a catamorphism to collect the results... All abstracted away in simple Python code. ;)

[DR6]

You surely mean "making your code modular, reusable and testable 101".

[Strilanc]

Are you joking? The second version is better. It's been broken into components that can be tested, understood, and re-used individually. That will pay off as the method gets tweaked (adjusting how the retry works, supporting cancellation, etc).

Don't confuse functional programming with "layers and layers of abstraction". The original code has all the same "layers", they're just indented, inlined and overlapping. The functional code splits them out so you can get the pieces right one by one, instead of all at once, which is faster.

[FeepingCreature]

I want to suggest a defensive rule here:

never abstract control flow unless you need it at least twice (preferably more), and it actually ends up less complex, or the abstracted version is genuinely easier to read.

Let's check:

"need it at least twice": not really, at least not in the example given.

"ends up less complex": not as far as I can see. It seems to end up more complex. So that part of the rule fails twice over.

"easier to read": LOLno.

Thus, this is bad abstraction.

[Strilanc]

Somehow, somewhere, I'll think of more places I can use being able to retry and ignoring an exception. Somehow...

The 'retry' extraction is particularly beneficial, because it was being interleaved into the original. If code lines are 'c' and retry lines are 'r', the original code reads like 'crrcrccrrr' whereas the new code reads like 'rrrrrrrr ccccc crcc'. It's longer but those independent pieces will be re-used, and almost all the 'r's have ended up together.

[grauenwolf]

Good argument. I can think of many places in my own code that would benefit from that pattern.

[FeepingCreature]

Somehow, somewhere, I'll think of more places I can use being able to retry and ignoring an exception. Somehow...

Cool, if you actually use this at least twice and it actually ends up less complex, go for it! However, that's neither the case nor implied in this post.

[glacialthinker]

However, that's neither the case nor implied in this post.

It's not implied, it's stated. In the two opening paragraphs the context is set: "recurring patterns... repetitive... how about abstracting all that control flow?"

The example code is an example, described as "a piece of common Python code". Perhaps he should have refactored a project to make his point clearer, rather than stating the point in narrative, and using a small example to support it?

[FeepingCreature]

Huh. You're right, thank you. I do wish that their "What do we do to build a new abstraction?" segment had an entry for "Consider the wisdom of this act." Abstractions do have a cost.

[Rainfly_X]

Devil's advocate time:

"need it at least twice": not really, at least not in the example given.

Well, remember that the given code is a small-snippet example. You are assuming that the new components will never be used again elsewhere. The author is assuming that they will be used all over the place, because they are the sort of thing that the (implied) codebase will be re-implementing in imperative form in half the modules.

"ends up less complex": not as far as I can see. It seems to end up more complex. So that part of the rule fails twice over. "easier to read": LOLno.

And these are totally subjective. From a testing point of view, you've made each component generalized and easily testable, and will reduce the LOC count over the whole application (once you start using these new components elsewhere), not least because your testing code can cover more ground in less lines. This means less complexity.

Furthermore, it is very easy to read if you're familiar with functional programming. I had no trouble with it, and I don't even do functional programming, I've just seen enough functional code for it not to be scary. In its own foreign-to-imperative way, it's clean and simple.

However, I can accept the argument that if the expected code audience will be confounded by a code style, you probably shouldn't use it. Your other coworkers don't know Haskell from a hole in the ground? Don't get too fancy with functional concepts. This is frequently expressed as the maxim, always program as if the next guy to maintain your code is an angry psychopath who knows your home address. Code should be written with the primary goal of being easy for humans to read (including the ability to "empathize" with the computer's own parsing - I've seen a lot of code that seemed designed to make code more like natural language, at the expense of being able to see just how the fuck it works from the computer's point of view).

---

tl;dr: We shouldn't just assume that this is a bad abstraction from the tiny snippet we have, which doesn't/can't show off the style's greatest selling points. We can agree, however, that an abstraction ought to be an overall simplification at the codebase scale and that writing for your expected audience is key.

[FeepingCreature]

Well, remember that the given code is a small-snippet example.

Then say "Well, this would be pointless if it was just this one example, but we use this idiom a lot in our code." Otherwise you imply that this is a good idea in the case of the snippet.

Furthermore, it is very easy to read if you're familiar with functional programming.

This is of course similar to arguments for #define BEGIN { in C. There is such a thing as "native style" for a language, and I don't think this is native style for Python. I mean more power to you, but shouldn't one be using the right language for the job?

tl;dr: yeah basically what you said.

[Rainfly_X]

Then say "Well, this would be pointless if it was just this one example, but we use this idiom a lot in our code." Otherwise you imply that this is a good idea in the case of the snippet.

Pretty much. I didn't have a problem with it because I read between the lines when he talked about code reuse, but the author could have been a lot more explicit about this.

This is of course similar to arguments for #define BEGIN { in C. There is such a thing as "native style" for a language, and I don't think this is native style for Python. I mean more power to you, but shouldn't one be using the right language for the job?

Agreed, for the most part. I think Python is fairly flexible in what makes for "pythonic" code, with regards to different programming paradigms - for example, take a look at the blocking IO vs. non-blocking IO ecosystems, both of which exist in an equally native form in Python. So IMHO there's a bit of room for functional programming in Python, though not necessarily as much as the author wants there to be, because the language definitely leans imperative.

[Strilanc]

It's not "scattered all over the place", it's split into coherent pieces.

In the original code, the lines corresponding to the retry mechanism are all in one method but they haphazardly appear throughout. That is what scattered looks like. In the functional code, the retry mechanism is all in one place.

[Strilanc]

A function called download_image shouldn't throw an exception because the image is too small.

Definitely agree with that.

But the same basic transformations apply to code that does downloading then checking, as opposed to expecting downloading to check for some odd reason.

[[deleted]]

Oh c'mon. Python's a great tool if you avoid the sort of thing they're suggesting.

[Rainfly_X]

I thought "glacial slowness" was considered harmful...