How to implement isValid correctly?

Question

I'm trying to provide a mechanism for validating my object like this:

class SomeObject {
 private $_inputString;
 private $_errors=array();
 public function __construct($inputString) {
 $this->_inputString = $inputString; 
 }
 public function getErrors() {
 return $this->_errors;
 }
 public function isValid() {
 $isValid = preg_match("/Some regular expression here/", $this->_inputString);
 if($isValid==0){
 $this->_errors[]= 'Error was found in the input';
 }
 return $isValid==1;
 }
}

Then when I'm testing my code I'm doing it like this:

 $obj = new SomeObject('an INVALID input string');
 $isValid = $obj->isValid();
 $errors=$obj->getErrors();
 $this->assertFalse($isValid);
 $this->assertNotEmpty($errors);

Now the test passes correctly, but I noticed a design problem here. What if the user called $obj->getErrors() before calling $obj->isValid()?

The test will fail because the user has to validate the object first before checking the error resulting from validation. I think this way the user depends on a sequence of action to work properly which I think is a bad thing because it exposes the internal behaviour of the class.

How do I solve this problem?
Should I tell the user explicitly to validate first? Where do I mention that?
Should I change the way I validate? Is there a better solution for this?

UPDATE:
I'm still developing the class so changes are easy and renaming functions and refactoring them is possible.

• Steven Evers: The other/better approach is to constrain inputs such that the object - and thus your application - cannot be in an invalid state.

Answer 1

There are two ways to get around this issue:

1. To set $_errors to an error indicating that the validation has not happened yet, or
2. To go "completely stateless": make isValid return the array of errors, and check the size of the array to determine if the validation succeeded or not

The first path is easier for an existing system: making this change is a lot cheaper, but interaction with your object remains statefull, in the sense that it relies on the order of calls.

The second path may be harder to integrate, but if it is applied consistently, it makes your API easier to learn. Different languages provide different ways of addressing the need to return multiple items from a call. In languages that allow passing by reference or by pointer, validating function often returns a boolean, and a placeholder for errors is passed by reference on the side.

• Songo: Thanks for the reply. I think I'll go for the second solution since I'm still developing the class. However, what should I rename isValid() to? coz this way the user might expect it to return a boolean IMO.
• Sergey Kalinichenko: @Songo Assuming that you are talking about the second approach, I would rename it to validate, and pass the input string to it.

Answer 2

Why would you let a consumer of your class create an object in an invalid state? I would design the class such that if it is ever asked to enter an invalid state (either by bad constructor parameters, or asking for an invalid mutation) it throws an exception and does not change the state.

• Songo: well I see your point, but I can't see how I can implement it in my example. The thing is I'm trying to parse a file which has 3 segments (header, body & trailer). So I thought I'll create an object File that has 3 objects Header, Body & Trailer. The file would parse itself to generate the 3 strings for the header, body & trailer. Each one of the parts would then validate itself using the corresponding string. How can I modify my code to reflect what you say, please?
• Songo: In addition there may be several reasons for each string to fail validation (several regular expressions) and I need to save them all.
• David Pierre: You can use an intermediate Builder object, responsible of the construction of the final object and of tracking the errors alongside. Then you can have a build() method that throws if Builder#isValid() doesn't pass and SomeObject is always created correctly.

Answer 3

Make isValid equal something by default that is only changed when it is is checked... When calling getErrors check for that default value and return an exception if it is there... That way, getErrors won't work unless isValid has been run first.

Answer 4

Make $isValid an instance varible.

Do the error check in the constructor including populating the array.

Then isValid() function only returns $isValid.

Doing the error checking in the constructor time-decouples getErrors() from isValid()