1

I have this if else structure and I want to refactor the else out since I want to clean up the code a bit. Only I am not sure how I can refactor the else out.

if ($validator->fails()) {
   return Redirect::back()
       ->with('error_code', 5)
       ->withErrors($validator->errors())
       ->withInput();
} else {
    // do something if validator does not fail
    return Redirect::back();
}

Anyone has any idea how I can refactor the else out?

Many thanks in advance!

2
  • 1
    You might want to check out codereview.stackexchange.com rather, though it's not clear how the code would be cleaner without the else. Commented Mar 1, 2017 at 11:45
  • Refactoring will be useless here. Commented Mar 1, 2017 at 11:46

3 Answers 3

4

In this case you dont even need the else statement, since you have a return in your IF. If the validation fails, it will return and never arrive to the Redirect::back(); So use it like this:

if ($validator->fails()) {
   return Redirect::back()
       ->with('error_code', 5)
       ->withErrors($validator->errors())
       ->withInput();
}

return Redirect::back();
Sign up to request clarification or add additional context in comments.

Comments

2

If you like to have a single return point at the end of your methods:

$back = Redirect::back();
if ($validator->fails)) {
    $back->with('error_code', 5)
        ->withErrors($validator->errors())
        ->withInput();
}
return $back;

Comments

1

If you want to minimize it as much as possible you can also remove the brackets of the if-statement. In PHP and in many other languages, if there is single line execution in a statement, you can remove the brackets.

So the minified version would be:

if ($validator->fails()) 
    return Redirect::back()
       ->with('error_code', 5)
       ->withErrors($validator->errors())
       ->withInput();

return Redirect::back();

You could also rewrite it using Ternary Operator in the following manner:

return $validator->fails() ? Redirect::back()->with('error_code', 5)->withErrors($validator->errors())->withInput() : Redirect::back();

Lastly I would like to point out that the best way to write code is not necessarily to write it as minified as possible. My example with the Ternary Operator above is a good example, does it really make it more readable to minify it like that? No.

Refactoring this type of statements should be done to make it easier to see and read what the statement actually do. Its not necessarily a bad thing to have an else {} the way you have in your original example.

2 Comments

Hey I agree mostly with what you're saying. I do not try to minify my code as mush as possible I simply try to keep it as clean as possible with a focus on readability! :) And having else statements makes code hard to read (imo) and I also think that having a single return point at the end of your methods/functions is a good habbit. That's just my opinion :)
Great whatever fits you! :) Just added that comment for future readers. Sometimes people try a bit too hard with their refactoring and you see like 4 nested Ternary If statements. Hope you enjoyed the answers. Cheers.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.