PHP OOP Theory - Error checking inside or outside of the class?

Question

I'm transitioning to using OOP for all my projects, historically everything I've built has been pretty small and OOP didn't seem to be an efficient choice, but now with large projects it is. However more recently I've been coming across more and more "best practice" questions that I have and I can't find the answer for.

For example, imagine I have the following:

class numbers{

    function __construct($number){
        $this->number = (int)$number;
    }

    function add($add){
        $this->added = $this->number + $add;
    }

    function multiply($multiply){
        $this->multiplied = $this->number * $multiply;
    }

    function minus($minus){
        $this->minused = $this->number - $minus;
    }

    function number(){
        return $this->number();
    }
}

Now let's say that I want to apply add, then multiply and then minus. Each stage can possibly fail (I didn't include that in the example, but imagine it's there). This is where my problem lies, should I do:

$numbers = new numbers(8);
if($numbers->add(7)){
    if($numbers->multiply(6)){
        if($numbers->minus(7){
            echo $numbers->number();
        }else{
            echo 'error minusing';
        }   
    }else{
        echo 'error multiplying number';
    }   
}else{
    echo 'error adding number';
}

Or should I have that part in my constructor, like:

class numbers{

    function __construct($number){
        $this->add(6);
        $this->multiply(9);
        $this->minus(7);
        if($this->error){
            return false;
        }else{
            return true;
        }
    }

    function add($add){
        $this->added = $this->number + $add;
        if(!this->added){
            $this->error = "couldn't be added";
        }
    }

    function multiply($multiply){
        $this->multiplied = $this->number * $multiply;
        if(!this->multiplied){
            $this->error = "couldn't be multiplied";
        }
    }

    function minus($minus){
        $this->minused = $this->number - $minus;
        if(!this->minused){
            $this->error = "couldn't be minused";
        }
    }

    function number(){
        return $this->number();
    }

    function error(){
        return $this->error();
    }

}

and then simply do:

$numbers = new numbers(5);
if($numbers){
    echo $numbers->number();
}else{
    echo $numbers->error();
}

Sorry if the example is long winded (also ignore the errors, I wrote it here just to outline what I'm trying to do, this isn't code I'm using...) but I don't know how to phrase the question without an example. Basically, should I be doing checking for the errors inside of the class or should I be doing it outside when I call the class?

Answer 1

It almost always makes sense to enforce consistency on updates inside the class (i.e. any time a method can mutate the internal state of the class, check the input). Then use exceptions to make it the caller's problem if that input is not well-formed, or if that operation will violate some class invariant.

Answer 2

Error checking done inside of a class should take care of the possibilities where that class's method will fail. In other words if your numbers class had a divide method it should check to make sure the denominator is not a zero.

Error checking that is performed on the instance of a class (outside of the class) should take be verifying that the correct results of a property or class method are returned. Since typically this shouldn't happen with a well designed class you are normally checking for extreme cases.

However, when you start to have a class that is keeping track of multiple things such as your case I would normally separate the class even further. Create classes for handling your math functions and then have your numbers class use those math classes. This way you have a better separation of concerns, neater code, and easier testing.

Answer 3

Personally, I would stay away from what you're trying to do in the constructor in the second example. It hard codes implementation, and violates the information hiding principal. Otherwise without know what the class does internally, when you do $num = new Number(8);, it's hard to understand why you're getting 47 back... It also violates the no magic number principal...

As for the methods, they look fine to me. You might want to throw an exception in those cases or return a boolean. Which you choose is a factor of a lot more than is being discussed here (Such as your preference, what exactly the class is doing, convention, etc)...

Oh, and you have a few infinite loop issues in your class. function number() should be:

function number() {
    return $this->number;
}

And the same for error()...