PHP method return without value

Question

I'm in a phase of migrating to 100% OO php and I end up with lots of questions like these. So excuse me if you find it stupid, I'm just trying to make my OO programming practices good in the start so that I don't have to fix them later.

Is it OK to do this?

private function _some_funct($args) {
    // Some code here....
    if ($something) return;

    //Rest of code
}

Basically, there are 2 questions here.

1. Is it OK to use return with no value after it? I use it a LOT and haven't encoutered any drawbacks, but never saw it in somebody else's code. I only saw return false.

2. Is stopping functions this way OK generally or should I rethink my program's structure?

Example of using this would be CMS I'm making.

public function _init_engines() {
    $this->_session_engine = $this->_dep['SessionEngine'];
    $this->_login_engine = $this->_dep['LoginEngine'];
    if ($this->_dep['User'] == false) return;   
    $this->_security_engine = $this->_dep['SecurityEngine'];
    //Other engines go here.......
}

So I let system start session and setup session vars, then check if user is logged in. If user isn't logged in LoginEngine takes care of that and loads 'Login' module. Once Login module is loaded, I don't want other engines to be instantiated since they are not needed. I could have used die or exit but that would stop whole script from executing. Like this, My script finishes executing, and other things that aren't engines, like benchmarks and some other stuff still get executed which is just what I want. Again, should I reconsider my logic here or is this way OK in your opinion?

Answer 1

return with no value is OK if:

1. You interface says that explicitly - docblock states @return void (if no result value needed in the function) or something like @return sometype|null where sometype is string, int, etc.
2. Code that uses return values from such functions/methods checks isset() before using the returned value - this can lead to bugs if people using your code (or yourself) forget to check it.

return with no value is NOT OK if:

1. Error occured - use exceptions instead. Exceptions are much more easier to deal with because you can:
   1. catch them anywhere in the outer scope, even globally
   2. pass nice error messages
   3. extend exception classes to provide additional functionality
   4. check exactly what type of exception was thrown
2. Some particular result type is expected - in that case you may consider some default "empty" result or, again, use exceptions if something gone wrong - it is still much better than isset because it is harder to forget - exception is a "loud" way to say that an error occured.

And please, don't ever use die() or exit() to "handle" errors - it is a very bad practice to show technical details of errors to users.

As for your _init_engines() method - it is really hard to say if it is right or wrong without knowing the rest.

If you are interested in good examples of OOP in PHP, I would suggest looking at Symfony.

Answer 2

This delves into the single return point vs. multiple return point religous battle, but her we go...

Using many returns without any value or real need for as execution flow control to avoid continuing in functions is pretty poor form. It's sort of in the magic 'goto' statement camp. Yes you can do it, but I'd vote no to actually doing it especially if it's not just a few returns early on to short circuit out of your function for a couple common cases.

Why not restructure your function so it has no return, but doesn't execute code it doesn't need to? If you find your functions/methods to large to easily restructure, it's probably a signal that means you might want to break them into smaller more concise methods/functions.

  public function _init_engines() {
    $this->_session_engine = $this->_dep['SessionEngine'];
    $this->_login_engine = $this->_dep['LoginEngine'];
    if ($this->_dep['User'] != false){ 
         $this->_security_engine = $this->_dep['SecurityEngine'];
    } else if (){
       //Other engines go here.......
    } 
    //no useless return needed
  }

Answer 3

As others said, there's no drawback in using return;. You usually see return false; because it's usually preferable to return something meaningful, when possible, to determine what the function did. If you are in the condition of "whatever it did, it's ok", then return; is perfectly acceptable.

Regarding the multiple return, I prefer using multiple exit points when conditions are sufficient to determine that the execution should not continue. For example, if you require a User ID and you don't receive one, maybe there is no point in checking the rest of data. In such case, you might end the function immediately with something like return RES_INVALID_USERID. In such case I prefer multiple return than a chain of if..else (even worse when they are nested).