Hacker News Reader

Comment by WesolyKubeczek | Hacker News Reader

Does this church still exist?

Sure, I'll play devil's advocate!

> Guard clause – exit a function early if certain conditions are not met. This makes the rest of the function easier to write (no nested if-statements).

If you have:

    if(!isUserPort(strPort)) return;

You're gaining knowledge about input, and then immediately throwing it away (assuming the 'happy' path.)

Later when you use strPort, do you check it again? If you pass it to another function, do you check it there too?

"Parse, don't validate" is the name that's been given to the following:

    case parsePort strPort of
        NotAPort -> ...
        RootPort port -> ...
        UserPort port -> ...

It's more nested, but now you can pass your UserPort around and not check it too many or too few times.

Unless your parsePort returns a specialized type which then can be enforced by the compiler, and all your other APIs use the specialized representation, it's sort of "six of one, half a dozen of the other" situation because if you're throwing your strPort around, you'll be parsing it again and again and again.

And I can achieve what you are showing me just the same by putting my guard clause up the stack, closer to the main program or whatever.

Not all data is so clear cut, your guard clause may be guarding against a numeric struct member whose value is out of range for this one function to operate.

Which is why I usually have a few different wrenches for each size of nut I may encounter. Not dunking on your approach, let a hundred flowers bloom and all that.

I’ve definitely run into its members recently. They tend to have other strange beliefs, like “methods can’t have more than four lines” and “you can instantiate an object only in a factory service”.

I didn't know the Clean Code church was also preaching the single point of return. Single point of return used to be big when Structured Programming was this "newfangled stuff". I'd rather expect some CS professor types smelling of mothballs and shambling about in empty hallways.

I think it's still required by by the MISRA-C standard, so if you write code for cars and some similar industries, you may get forcefully dragged into that church.

Yep, it may come from that, or maybe as it often happens someone wrote a best practice in the coding style rules, something reasonable like "try to avoid returning from the middle of a function".

People didn't understand that, too vague, hard to enforce. Someone else decided to make it clearer, "try to avoid multiple returns inside a function".

Then someone else comes and decides to tidy up the coding style document, put it in imperative form: "All functions should have a single return statement".

Give it a couple of years, people forgot why the advice was there in the first place and now they just blindly apply an insane rule, teach that to new hires and enforce it in reviews. All functions now look like big arrows and every once in a while the max line length rule has to be relaxed a little to allow for all those levels of indent.

Maybe that's how it entered MISRA-C in the first place.

I'd argue that in languages with manual resource management, you should be very careful how you return in the middle. Say you allocate something, or initialize communications with a device before a loop begins. In the middle of the loop, you find your result early and want to return it.

But your "return" should also have all deallocations performed! Also if you open communications to some external devices which tend to be notoriously stateful, you need to bring them back to a usable state by either completing commands sent so far, or resetting them, or whatever is needed.

If there is nothing analogous to "defer" statements or "finally" clauses, you will be in a world of pain if you sprinkle return statements without paying attention to all details.

Thus I can see a point of such a rule being in MISRA-C or something like it. Guess it's better to enforce an easily checkable rule (prone to birthing monsters) than educate all people properly.

Notice I purposedly used "reasonable" and "in the middle of a function". I know where this rule comes from. It's how it evolved into a blind prescription that is a problem.

Yep. When investigating a solution for a government-adjacent organisation I've been handed a standard which specified single return per function.

A recommendation likely lifted from one of these sources.

MISRA 2004, Rule 14.7: "A function shall have a single point of exit at the end of the function.", required - https://www.ibm.com/docs/en/rtr/9.0.0?topic=review-code-misr...

MISRA 2012, Rule 15.5: "Only one exit point should be defined in a function.", advisory - https://www.ibm.com/docs/en/rtr/9.0.0?topic=review-code-misr...

HN users cordenr and bfrog say it was dropped in MISRA 2023: https://news.ycombinator.com/item?id=38680587 and https://news.ycombinator.com/item?id=38704631 . Both in a recent thread on MISRA 2023 at https://news.ycombinator.com/item?id=38674158 .

FWIW, HN user FirmwareBurner in that thread links to the "Embedded System development Coding Reference guide" version 3.0 (2018) from the Software Reliability Enhancement Center, Japan at https://www.ipa.go.jp/publish/qv6pgp00000011mh-att/000065271... which says

  M3.1.5:
   A function shall end with one return statement.
   A return statement to return in the middle of processing shall
   be written only in case of recovery from abnormality.

I was not aware of the changes in MISRA-2023, thanks for informing me!

Also, that's great news to hear in case I ever want to go back to the automotive industry. :)

Well, aren't early returns in guard clauses "recovery from abnormality"?

You could flex some language lawyering to define "abnormality"...

I believe that's exactly it. An exception to the single return rule to allow for guard clauses. Wording is maybe a little too obscure though.

I don't know.

Here is the only relevant example I found:

  p = X_MALLOC(sizeof(*p) * NUM);
  if (p == NULL) {
    return (MEM_NOTHING);
  }
  ...
  X_FREE(p);
  return (OK);

It's clearly abnormal.

Something like a:

   if (get_size(obj) == 0) {
       return empty_case;
   }
   ... do more complicated code here ...
   return result;

does not feel "abnormal", which are things you likely want to log, given:

   Logs should be output not only when an abnormal condition
   is detected, but also at the timing of, such as, data 
   communication with an external system.

Definitely. Let's wait if some adept shows up. I met plenty IRL.

One specific case I have in mind I think it came from the company mostly hiring fresh graduates and seniors indoctrinating them with their blind and badly aged structured programming rules.

Reply on news.ycombinator.com