I have a class that does something, and when it's done it returns true
, but if any of the various things go wrong it will return false
. Now my question is, should status messages for such events be kept inside a class (Example1) or outside the class (Example2) where class maybe only provides error code to help distinguishing what happened.
class Example1 {
private $var;
private $status;
public function doSomething($var) {
$this->var = $var;
if (!is_numeric($this->var)) {
$this->status = 'Please provide a number';
return false;
}
if (empty($this->var)) {
$this->status = 'Please fill the field';
return false;
}
$this->status = 'Ok, you submitted a number, cool.';
return true;
}
function getStatus() {
return $this->status;
}
}
example 2:
class Example2 {
private $var;
public function doSomething($var) {
$this->var = $var;
if (!is_numeric($this->var)) {
return false;
}
if (empty($this->var)) {
return false;
}
return true;
}
}
Example 1 seems to me more convenient to use, code reads like a poem, but at the same time seems less reusable, depending on what you use class for, you might want success/error messages to have different syntax.
So basically my question would be what's the usual practice?
First example: you hard code error messages which is... bad. And you can't see the error message if you don't check $object->status
later.
Second example: if something goes wrong, you know it but you don't know why.
I suggest avoiding both these ways and throwing exceptions for a more object oriented approach (I guess that's what you want, since you're using classes :).
It's completely up to you. If error messages are easier for you to understand and help you debug your code, then by all means go for it. Coming up with error codes for different errors will probably just slow down the development process, unless you want to hide the true errors from users (but in that case you have several options, one of which is to not print errors to screen at all if users might see them).
It's not what you are asking, but it's generally considered bad practice to have multiple return statements in one function. The problem is that it makes it harder to know where you exit the function, which makes it not clear what is executed. In your example, if the first test succeeds, the rest of the code is not executed. What would happen if you set some attributes afterwards ? Sometimes they will be set, sometimes they won't. To get rid of this problem, it's better to have only one return statement. More code will be executed, and you'll have to change the way you are coding a bit in order to get used to it.
Here is an example of what I mean, with the code you provide :
public function doSomething($var) {
$this->var = $var;
$result = false;
if (!is_numeric($this->var)) {
$this->status = 'Please provide a number';
}
else if (empty($this->var)) {
$this->status = 'Please fill the field';
}
else{
$this->status = 'Ok, you submitted a number, cool.';
$result = true;
}
return $result;
}