good luck

@carusogabriel What bugs does this fix? What problems does it resolve? Does this simplify anything for our users?

@sisve Is a mantra that I learned and follow 🙏:

There is no else!

We can simplify ways (I don't know if it is the correct word here) in our code, instead of two, just one. If we've already exited the code, nothing else will be executed, soo, why else? If it exited the code, continue to execute, no enter in else statement.

Relevant: #21862, #21767, #20900, #20058, #19475, #19187, #19025, #19026, #19028, #19029, #15337

I just can't get why he doesn't merge it. It's cleaner, reduce complexity and other stuff 😞

It's cleaner, [...]

That's your personal opinion.

[...] reduce complexity [...]

That's your personal opinion.

[...] and other stuff

That's slightly vague...

@sisve About reducing complexity: https://www.slideshare.net/guilhermeblanco/php-for-adults-clean-code-and-object-calisthenics (slide 33 and beyond) and https://www.ibm.com/developerworks/rational/library/3101.html (Alternatives section)

The slideshow has 15 slides with code changes, and on slide 48 claims that some of the changes has reduced the cyclomatic complexity. However, the presence of an else-clause does not affect cyclomatic complexity at all. So the argument about reduced cyclomatic complexity is not about the else-clauses, but the other changes he does over the 15 slides.

To be clear; I am not saying that your suggestions will make the world a worse place. I'm just pointing out that these things are primarily opinion-based, and as such, may not conform to the mind of The Merger.

One would imagine that having some kind of coding guidelines for these things would help in the future...

No problem, opinions and points something diverges 😅

About Coding Guidelines: very interesting 🤔

Don't typically merge these. Only merging this one because it has only 1 small merge conflict against master. Thanks 😅

Nice... Now others with PRs get to do extra work, because of some personal preferences...

And furthermore, code-folding is no longer possible. Enjoy.

@taylorotwell In my humble opinion, these syntactical preferences should either be ignored (as you have done in the past) or articulated in a standard that is posted clearly to https://laravel.com/docs/master/contributions#coding-style . These are frivolous changes that waste peoples' time and create unnecessary merge conflicts (as can be seen in this specific instance).

Yeah these changes may seem to be useless, but they help us undertand the framework, thanks @carusogabriel

Why is a function more difficult to understand when you have code paths clearly defined?

If I want to determine what happens when an if statement fails, it's easier to scan for an else statement than to scan for braces, especially in nested code blocks.

Previously, if I wanted to know what a function returns, all I had to do was scan backwards for a return statement and check if it was in an if/else block. Now I need to scan the entire function for returns that may or may not be in conditional blocks.

Making code less obvious, less pedantic or less explicit does not make it cleaner or more readable, it does the exact opposite.

The next logical step here is to just use ternary statements for all of these methods like return $condition ? $success : $failure;. That's less code, so it has to be better, right? It's also a lot more readable because you only have one return statement instead of having multiple statements and having to figure out when each of them is executed.

This!!! ^ Very well stated, @36864 👏

And @jleonardolemos , I am very interested in your explanation as to how changes of this nature "help us understand the framework". You might start by refuting what the astute gentleman stated, just above.

Guard clauses are good thing to keep code clean and readable. They are much more useful for simplifying nested conditionals, but it's not bad to keep same code style over all project.

Refactoring with Guard Clauses by Martin Fowler

This PR was submitted following a mantra that @carusogabriel personally learned and follows: "There is no else". Emphasys: personally. I know of a lot of devs who would consider this mantra dangerous. It is "using something that is supposed to be relative (case by case) as absolute and then forcing others to adhere to it".

@36864 said:

Making code less obvious, less pedantic or less explicit does not make it cleaner or more readable, it does the exact opposite.

I thought that Laravel was proud to be written in clean, readable code, since that is the mantra of folks close to the framework. Hence my failure to understand why @taylorotwell finally caved in, after ignoring such requests for a while.
#19025 (comment)
#19028 (comment)
#15337 (comment)
Etc...

@Stretsh

Nice... Now others with PRs get to do extra work, because of some personal preferences...

What do you mean with 'extra work'?

@a-komarev
I'm on board with guard clauses to prevent nested conditionals. This is not what's being done here. An else statement is not a nested conditional.

Ultimately, I'm okay with either style being used. What I'm not okay with is the style being changed for no reason other than someone thinking it's "more readable", and making the code inconsistent.

Inconsistency is what really annoys me here. If early returns are the holy grail and else clauses are the devil, why were elseif statements kept?
Either choose early returns with no else statements, or proper code path blocks with else statements. Going at it half-assed and leaving the job half-finished is just a waste of everybody's time.

@36864 I think because it's much harder to remove elseif statements. It's really complex work to refactor all the stuff at once and more harder to review such PRs. If you are so mad on this inconsistency and elseif statements not being touched - you could make PR which will do the next step to better code.

much harder to remove elseif statements

What? Let's look at a real example from this PR: (comments removed for brevity)

 protected function getDsn(array $config) 
{
  if (in_array('dblib', $this->getAvailableDrivers())) {
    return $this->getDblibDsn($config);
  } elseif ($this->prefersOdbc($config)) {
     return $this->getOdbcDsn($config);
  } else { 	
    return $this->getSqlSrvDsn($config); 	
  }
}

This was refactored to just remove the final else statement. Removing the elseif statement would have required the following additional changes:

• delete the letters [e, l, s, e] from elseif

If I were to make a PR regarding this, it would be to revert the changes made.
Guard statements are useful for two things: Preventing excessive conditional block nesting and short-circuiting execution when invalid data or an invalid application state is detected.
Another tool that's quite useful for preventing nested conditionals is the elseif statement. You get all the joy of having separate conditions without the indentation hell of multiple if/else blocks. Whether guard statements are more readable than chained elseif blocks is a matter of opinion.

The only file where removing the else block can make sense, under the guise of "we don't need it because guard statements" in this entire PR is Migrator.php since the if statement is handling what could be interpreted as an error (attempting to rollback migrations when there are no migrations to rollback).

@36864 Good catch. You are right, in this example its pretty clear. I was talked about making changes in all framework code where elseif is met.
Want to try your luck and check if this change will be merged? 🤣

The next thing you know, we'll be arguing about tabs vs. spaces! Oh, wait... The only sensible argument in this thread is that this preference needs to be standardized, one way or another (and so far, only three people, myself excluded, have advocated for as much). I care a lot more about making useful contributions to the code-base than reverting each other's work. This one's on you, Taylor. You set a precedent with this one, and it smacks of favoritism.