views:

218

answers:

7

Is this bad? Or this is very common in PHP framework?

For example, in the parent class, there is a save() function which returns the number of rows affected in the database. Then in the child class, I override this function to do some pre-validation, and also would like to simply return a success/failed boolean value.

+3  A: 

Yes it is bad. It will be hard for the client of that method to be know that to expect. UNLESS, the method is designed to handle it (which in this case, it does not seems like).

I will strongly advice against it.

NawaMan
+3  A: 

You should be overriding the underlying code, not the contract.

So, no, it's not good to change the return values (which is part of the contract).

strager
+2  A: 

While there is nothing enforcing return types in PHP, it is a good idea to remain consistent with overridden methods because to do otherwise means that you will increase coupling and decrease encapsulation.

Think of public methods and their return types in terms of outlets and appliances. Any appliance should be able to plug into any outlet and every outlet should output the same amount of energy to each appliance, unless of course the outlet has been specifically designed for a specific type of appliance. What would happen if someone decided that green outlets should output one amount of energy and red ones another. Suddenly you'd have to always be concerned with the specific class of outlet that your appliance is dealing with.

Designing public methods and properties is exactly the same and overridden methods and properties should always behave consistently regardless of the context in which they are accessed.

Noah Goodrich
+4  A: 

I agree with consensus that it's bad to change the type (or even the meaning) of the return value.

Here is an illustration how it will be bad:

Let say you have a function that accepts a string and a 'Writer':

function printThroughWriter(String $text, Writer $writer) {
    $count = $writer->write($text);
    print "Writer '".get_class($writer)."' printed $count characters.";
}

And here is the original 'Writer' class:

class Writer {
    public function write(String $text) {
     print $text;
     return strlen($text);
    }
}

What if we pass a 'LazyWriter' instead to the function:

class LazyWriter extends Writer {
    public function write(String $text) {
     return true;
    }
}

The assumption that the return value of the Writer::write() is the number of characters in printThroughWriter has been broken, thus causing inconsistency.

Lukman
+2  A: 

I agree, that this is generally a bad idea.

If you desperately need to pass another value out of the function, other than it's return value, you can add a pass-by-reference parameter. Or, in case of error messages, use Exceptions.

Atli
+2  A: 

The idea of inheritance is to allow clients to use the sub types of a class interchangeably. If you are changing return types of sub types then you are destroying the normative utility of inheritance and will just end up creating confusion: "Here I take a type of Collection, but is myCollection.add() going to return me a boolean or the element that was added or something else? I have no idea! I can't do anything anymore with this type! I'm going to have to make a method for each sub type in order to handle this problem!"

mlaw
+1  A: 

You can return a more specified type, such as an instance of a descendant of the return type from the overridden method, without breaking a contract.

If the overridden method returned a boolean, the overriding method could instead return a number if 0 return value corresponded to the conditions that would cause the overridden method to return FALSE and non-zero values corresponded to a TRUE return value. This could still potentially cause problems if someone used the identical comparison operator (===) on the return value, but you typically only see this when (e.g.) a method might return both FALSE and 0 under different conditions.

In your example, the overriding return type is not more specific than the overridden return type, so you shouldn't do it.

outis