Both listings have early returns, it's just that the elses in the first listing aren't really needed

Possibly older devs write like this because other languages (mostly outside of web stack AFAIK) can consider that a single exit point is safer, same goes for making sure (generally speaking) any 'if' has an 'else'. For example see MISRA-C-2004.

For some reason the younger devs in the team prefer to write code this way ☝️, but the truly enlightened devs will write code like this:

const foo = bar =>
  bar > 0 ? 'YES'
  : bar === 0 ? 'MAYBE'
  : 'NO'

or like this:

import { always, cond, equals, gt, T } from 'ramda'

const foo = cond([
  [gt(0), always('YES')],
  [equals(0), always('MAYBE')],
  [T, always('NO')],
])

Use a single return!

p.s. also be careful of ageism ;)

I'm fine with any of those, just don't mix curly brace styles! :-P

Option 1, one return at the end. Best for debugging.
Option 2: early returns. I consider valuable when you have a simple condition that could be evaluated at the beginning and the rest of conditions are more complex, so you can save some precious cpu time haha

I tend to use the second example a lot in code that isn't large or repetitive enough to be abstracted, and I don't need to return a value immediately. That way I can also intercept the value before it's returned a couple times, and run other checks if necessary.

The final example is a little more elegant, but makes presumptions about the situation. Ideally we'd all do that, but then we'd also have one-off functions for logic that's singular in occurrence.

Returning from functions at will is against programming principles. The older programmers knew better.

Well made point!

But isn't the whole point of a function to abstract a thing that's done frequently in order to avoid copying the code, and making it easier to modify it in one place should changes be necessary?

I'd say I don't care which way it's written, as long as the documentation is clear on what it does, the function name gives me a hint, and I can look at the code once, determine that it's appropriate for my use, and then (the best part!) completely forget about the implementation details.

So in some sense, all of these, and their counter-examples, and the obfuscated code submitted by other commenters, are fine because they get the job done.

And in some sense they are all undocumented FooFunctions that are going to add to the debugging load and the GettingJuniorDevsUpToSpeedOnTheCodeBase time.

And on the other tentacle, it's just a religious style war, or a hazing ritual for the JuniorDevs (unless there's a well documented, properly enforced, company-wide style guide). Bonus points for easy documentation updates so I cay say "I'm using this over here in IncrementalSolarPayback" so any changes to the function will know what might break.

I am not a proper Dev, so feel free to tell me I'm out of line... [But no, I don't buy the code efficiency argument, either throw more CPU at it or write it in assembly. 8*]

What about:

        function foo($bar)
        {
            $value = '';            

            if ($bar > 0) {
                $value = "YES";
            } else if ($bar == 0) {
                $value = "MAYBE"
            } else {
                $value = "NO";
            }



            foo( $value );
            return $value;
        }

The simple addition of foo makes the second form entirely reasonable.

Without knowing the history of the code it's hard to say why it's ended up at the form it currently has. It's way too easy to look at things in isolation and find fault with them, or draw conclusions about what is clean/bad.

I always try to avoid "else". I know Golang encourages the early return pattern

I spotted this over the weekend, and had a few thoughts, but didn't have time to chime in. Now that I have time, it looks like a lot of what I would have said has already been covered. So just a few quick thoughts that I hope will be helpful.

I've worked on various codebases with different styles. What's more "readable" or "clean" generally is more a function of what you're accustomed to looking at than any objective measurement. Codebases with a consistent style are far easier to work on than codebases that mix different styles, even if they're consistently ugly.

Focusing on stuff like bracing styles, if/else formatting, etc, is focusing on the smallest possible minutia of the codebase. Consider: If you could magically reformat all the code so that it's to your liking, what would you want to fix after that? Is reformatting the code actually stopping you from doing that next thing? (Spoiler: No.)

I totally feel your pain in having to look at code that you feel could/should be clearer/cleaner. Please know that I've been there. But when the more seasoned devs you work with all tell you the same thing, it's worth strongly considering their perspective.

It's said that safety regulations are written in blood. The preferences of senior devs are based on long years of bad experiences. Ignore them if you like, but don't doubt that you'll eventually learn the hard way.

        function foo($bar)
        { //why?

            if (...) {
                ...
            }

        } //why?

When I started reading, I thought the problem was that the inconsistency of the placement of braces...

You should probably push your boss to fire all those old “dinosaurs” who are writing garbage code, and just hire a bunch of juniors to come in and revolutionize everything.

I hope in another few years I’m lucky enough to get the axe. I wouldn’t want to hold my company back. Or maybe they should start refactoring now. I’ll quit on my own volition if they want to get our finance app reduced down to a single function expression.

personally i'd go with the less conventionnal :

        function foo($bar) 
        {
            if ($bar > 0) return "YES";
            if ($bar == 0) return "MAYBE";
            return "NO";    
        }  

..somewhere between ternary and switch disposition, i find it clearer, obvious and fautless performance-wise.
The point of concern is when another dev replace the return with more operations defeating the "escape" purpose of "return" statement.
So i can understand that the "else" are somewhat preventing falling through.
However, as $bar cannot at the same time be 0 AND superior to 0 AND something else, i'd say it's safe.

In terms of control structure it calls for a switch, however many people reject this form :

function foo($bar)
{
    switch(true) {
        case ($bar > 0):  return "YES";
        case ($bar == 0): return "MAYBE";
        default:
            return "NO";
    }
}