Hacker News Reader

Comment by bcrosby95 | Hacker News Reader

The first example is bad for reasons not related to ifs and fors. In general, if you can, if you have a "container" for something, you should write functions on the contained, domain-level "Thing" rather than the container with the domain level thing.

As an example I work with - Clojure. Sometimes I use agents. I don't write functions for agents, I write functions for things agents might contain.

Similar rules for Elixir. My primary domain level functions don't work off a PID. They work off the underlying, domain-level data structures. GenServer calls delegate to that where necessary.

This makes them more flexible, and tends to keep a cleaner distinction between a core domain (frobnicate the Walrus) and broader application concerns (maybe the Walrus is there, maybe not... oh yeah, also frobnicate it).

View on HN

Yeah. I think the given advice probably takes validation logic and floats it too high. It is of course nice to have early validation logic, but it is also nice when your functions don't mysteriously crap out with some weird error but instead shout a validation error at you.

Haskell solves this with newtypes, “here is a transparent container that certifies that you did the appropriate validation already,” that helps for this.

The advice that I really want to hammer into people's heads is, prefer “sad ifs.” That is, I will almost always find this

    if (something_is_wrong_in_way_1) {
      // fix it or abort
    }
    if (something_is_wrong_in_way_2) {
      // fix it or abort
    }
    if (something_is_wrong_in_way_3) {
      // fix it or abort
    }

more readable and maintainable than this

    if (things_are_ok_in_way_1) {
      if (things_are_ok_in_way_2) {
        if (things_are_ok_in_way_3) {
          // do the happy path!
        } else {
          // fix or abort thing 3
          // if fixed, do the happy path
        }
      } else {
        // fix or abort thing 2
        // if fixed, test way 3 again
        // if way 3 is good do the happy path, else fix it
        // ...
      }
    } else {
      // ...
    }

I feel like it's in human nature to focus on the expected case, I want everyone whose code I meet to do the exact opposite, focus primarily on the unexpected. Every “if” imposes a mental burden that I am keeping track of, and if you have to go to an external system to fetch that information, or you need to exit early with an error, I can immediately discharge that mental burden the moment I know about it, if the handling and the detection are right next to each other.

In short: avoid `else`.

Both what you mention, and also the case with :

   if (...) {
     ...
   } else { 
     return 0; 
   }

Which can become

   if (...) {
     ...
   }

   return 0;

Even better, if you're using a language with pattern matching (rust, any ML, haskell), just put it in a single match with the happy case on top, and the error cases below.

To do this, you need to encode the distinction between happy and sad path in your types. That's good for all sorts of other reasons, so it's not really a drawback.

Yes, pattern matching really helps with clarity, especially when there are multiple happy paths, like in a parser

What about `else if`? Not a fan?

Occasional vice for me.

Personally I only either use if statements that wrap a single line of logic, or a switch/case statement where each branch is a single line of logic.

Most situations are handled by guard clauses, exhaustive type switching, or switching on logical cases. Very occasionally do I have to break this practice for a legitimate reason, but this keeps functions simple to read and to reason about.

I occasionally am unable to avoid an else, but I will always avoid else if. It puts too much reliance on remembering how this condition interacts with the one on the if statement above it, which can often be many lines away. I’d infinitely rather use a case statement where all the alternatives are in an easily-understood table with every condition aligned on successive lines.

In functional languages like elixir there is no return though, and you are forced to use if-else structure

I tend to prefer a style more like:

    if (!...) return 0;

    ...

Because otherwise you can end up with lots of nested conditions which are very confusing to follow, even though they boil down to "do the thing if these are true, otherwise don't do the thing".

I usually move this up in an ETL process into a function I usually call "Pre_Flight_Checklist." Over the years this has gotten its own helper functions. For example, if I am going to use a file, I have function that not only checks for the existence of the file, but that it is a file. This function can also be fed an expected size range and date if "freshness" is expected. If something is out of whack, an error message will mention that a file was expected at /Adirectory/Bdirectory/Cdirectory/filename.ext, but only /Adirectory/Bdirectory was found, nothing under it. I believe in overly-explanatory error messages.

Now, I do run the risk of having things change out from under me, but putting all of the defensiveness up front allows the happy path and some alternatives to be more succinct in the code.

I keep finding new things to be paranoid about, though!

I at first had the opposite reaction, which is that I spend a ton of time fighting inverted ifs! But my reaction is not incompatible with yours. It's not, but it is related: I HATE reading:

if (!someCondition) { // short amount of code } else { // longer happy path }

The contextual overhead of having to invert a negative (i.e. !!someCondition) is just annoying.

I do agree that if (happy) { /* tons of code / } else { / I forgot how we got here */ } can also be an issue.

The key to the idea above is that you don't have any "else" clause. It's an early return, raise an exception, or modify the data so it's no longer bad.

Yes, that's why I said I realized they weren't quite the same situation, while adding another example of a situation where people create hard to follow structures.

> The advice that I really want to hammer into people's heads is, prefer “sad ifs.”

I think this is the first time I've heard that name for this idea. You'll find more under the term "guard clauses" - it even has its own Wikipedia page: https://en.wikipedia.org/wiki/Guard_(computer_science)

Yeah, same, also know this as guard clauses. But "sad if's" is kind of adorable sounding, so I might start using it.

pessimifs

depressifs?

TIL: another word for "guard clauses" is "sad if's".

I once had a CS teacher who hated early returns and break statements; they would always prefer the "nested ifs" approach. And they're far from being alone - many others believe that path is cleaner.

I'm with the parent poster, though: if you can use control flow to keep the happy path the unindented path, and you have syntax highlighting to help you show that each guard clause actually does interrupt the control flow, it can often be significantly cleaner.

I also like to think of it in terms of "railway oriented programming" (https://fsharpforfunandprofit.com/rop/ which is a must-read for anyone trying to wade into monads and functional programming IMO) - the "happy path" should be the path that it's easiest for the train to go down, namely a straight line down the page. Only give yourself the headache of a sharp turn when you're interrupting that path!

I don't mind early returns, but I'm infamously anti "break" and "continue" statements (and all my colleagues to date have disagreed with me and have found my stance hilarious and/or infuriating). My rationale is that they turn a loop into a mutant abomination that's a loop and conditionals all mushed up together. I also can't shake this gut feeling that they're crude and antiquated control flow mechanisms, only one step away from the fiery inferno of "goto". So instead of this:

    for thing in things:
        if thing.is_yellow():
            continue

        if thing.is_rainbow():
            break

        thing.thingify()

I'll do:

    things_iter = iter(things)

    thing = next(things_iter, None)
    is_rainbow_thing_found = (
        thing.is_rainbow()
        if thing is not None
        else False
    )

    while (
        thing is not None
        and not is_rainbow_thing_found
    ):
        if not thing.is_yellow():
            thing.thingify()

        thing = next(things_iter, None)
        is_rainbow_thing_found = (
            thing.is_rainbow()
            if thing is not None
            else False
        )

I’m with your colleagues I’m afraid. The intent of the all-mushed-up-together version is really clear. I had to spend a long time looking at the longer version to figure out what was going on. I know which I’d prefer to maintain if I had to.

Considering break and continue to be too powerful as control flow statements is a somewhat common opinion, in my experience. (I still use them, but only sparingly, and where I am quite sure that they're not confusing.)

That said, your second code block is unreadable to me -- even knowing what it does, it's just confusing. No way that that's more readable than making use of break here. If you don't want so many conditionals in the loop body, you can factor the first check out:

    no_yellow_things = (t for t in things if not t.is_yellow())
    for t in no_yellow_things:
        if t.is_rainbow():
            break
        t.thingify()

I almost never say this: you are wrong. Code needs to be maintained. Everyone can understand the first example. Everyone has to slowdown and reason about your second example. It is objectively worse. And your argument is flawed: break and continue are not the goto-like behavior that goto is criticized for as they are limited to the scope of that block of code.

The problem traditionally cited with goto is it can go anywhere (even into the body of another function where expectations on which variables are set may not hold).

If you don't like break and continue, how do you justify exceptions? The handling code for those may be in a different file or nowhere. Much closer to goto.

As others have said here, if you want to do it nicely and avoid break / continue, then do it in a proper FP way with chained limit / filter calls, rather than doing it per my example.

I accept your criticism, that my example is much harder to understand, and I admit that it's probably not the way to go in terms of maintainability. However, I'd still argue that my example is closer to an FP approach (even though it still uses a procedural loop), and that it communicates the logic in the same way that an FP approach does. In my example, it's clear that we're filtering out yellow (whereas with "continue", the logic is written in an inverted way); and it's clear just from reading the loop's header that we stop looping when rainbow is found (whereas with "break", logic to stop looping is being added at any arbitrary spot inside the loop's body).

Re: exceptions. I know that many more people object to them, than to break / continue (and whole languages make a point of not using them, most notably Golang). The pros and cons of exceptions is a whole topic in itself that I'd rather not digress into right here. Anyway, personally I'm ok with them. Firstly, they're for errors, and I feel that having weird control flow is sort-of justified for errors, whereas it's not justified for expected behaviour. And secondly, they're at least consistent in that they can occur anywhere, whereas break / continue can only occur inside loops (in Python, at least - in other C-family languages, "break" has different behaviour in different contexts, which is IMHO even worse).

Minimizing control flow in loops is a good idea, but if you can FP-ize the loop entirely that keeps it pretty readable IMO.

    things
        .iter()
        .filter(|t| !t.is_yellow())
        .take_while(|t| !t.is_rainbow())
        .for_each(|t| t.thingify());

That's the one I would prefer. If you're at all used to FP, this signals intent very clearly.

That's the nicest counter-example to my example, thanks for that! I wasn't familiar with take_while() (looks like that's Rust, and looks like Python has a similar itertools.takewhile() ), TIL a neat FP version of break. My example was quite trivial, it's not always so obvious how to break up a convoluted loop, but it should always be possible.

I'm afraid I'm with your colleagues.

I value being able to grasp what the code does at a glance while scrolling through. Your first snippet is entering my brain like as if without even reading at all, all at once and instantly.

Your second example absolutely does not and I'm sure you know why.

Surely you are joking? Your version is four times longer and repeats a large section of itself.

The way to avoid control flow here is to simply find the rainbow first, take the sequence up until then, filter by yellow, and thingify the remaining things.

    rainbow_idx = next(i for i, t in enumerate(things) if t.is_rbow())
    yellows = (t for t in things[:rainbow_idx] if t.is_ylw())
    for t in yellows:
        t.thngry()

Excuse formatting, cellphone.

Not sure this is any better than the obvious solution either.

Yes, I agree, that's the way to do it. Although it isn't always so obvious how to break it up into limit / filter steps, my example was quite trivial. But there should always be a way somehow.

Do they come from the times when structured programming was hot and new, SESE had been the buzzword of the year, and that doodad called "transistor" just had been invented?

The author called out the newtype solution specifically in the first paragraph:

> it could push the task of precondition checking to its caller, and enforce via types (or an assert) that the precondition holds

(Emphasis mine)

I recall avoiding multiple method exits (in Java) like the plague. They were hell to get through in a debugging session. But I guess we don't use debuggers much any more.

When I began writing C# I recall my other team members "refactoring" my code to be shot through with multiple exits instead of a my collecting variable, because "it looked nicer." When they asked me why I wrote it the way I did, I mentioned the Java debugger thing, and they kind of looked at me blankly and said, "Huh, never thought of that."

Times change.

Why is multiple method exits hell to get through with a debugger? I use a debugger for my C++ code all the time and I’ve never run into an issue because of multiple returns.

Setting breakpoints to inspect local state means you have to catch every single exit. Back then, if you had a thirty second to one minute round trip to get back to the method you were inspecting, it became... frustrating. You probably didn't have to deal with those kind of start-up round trips in C++. With C++ you'd pay on the compile time.

If you wrote the code to have a single exit, the cognitive load was also typically lower, as the control flow could be thought of as one way in, and two ways out (return or exception). Good disciple also followed "the jolly good idea of Demeter" or whatever it came to be called; that being to only operate on the state (or preferably, values) passed in, where possible.

I know this is a late response, but this still doesn’t really make sense to me. What stopped you from placing the break point at the start of the function, before the return values were hit and stepping through the code?

In my experience, having multiple exits is even better because I can check which edge case is being hit by placing multiple breakpoints, or a breakpoint at a specific exit I want to check. I understand the pain of having to step through the code to get back to a certain execution point, but I’ve never been in a situation where multiple exits caused such cognitive load as to hinder that process further.

Great insight. many programmers fail to think of their errors valid as values to get as an output. Golang got this right

Correct me if I am misunderstanding, you are saying the first example would be better if it was `walrus.frobnicate()`? Isn't that a syntax preference more than an issue with the point the author is trying to make?

> As an example I work with - Clojure. Sometimes I use agents. I don't write functions for agents, I write functions for things agents might contain.

I'm not following you. Do you have a better example?

Similar to the example of the OP:

  (def walrus (agent {}))

  ; good
  (defn frobnicate [w]
    (assoc w :frobnicated true))
  (send walrus frobnicate)

  ; bad
  (defn frobnicate [w]
    (send walrus #(assoc % :frobnicated true)))
  (frobnicate walrus)

It might make sense to also have a function that executes something like (send walrus frobnicate). But IMHO code should basically never look like the bad example.

Reply on news.ycombinator.com