Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
1  
The other/better approach is to constrain inputs such that the object - and thus your application - cannot be in an invalid state. –  Steve Evers Oct 11 '12 at 20:43
add comment

4 Answers

up vote 2 down vote accepted

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.

share|improve this answer
    
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. –  Songo Oct 11 '12 at 20:29
    
@Songo Assuming that you are talking about the second approach, I would rename it to validate, and pass the input string to it. –  dasblinkenlight Oct 11 '12 at 20:30
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
    
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 Oct 12 '12 at 0:42
    
In addition there may be several reasons for each string to fail validation (several regular expressions) and I need to save them all. –  Songo Oct 12 '12 at 0:50
1  
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. –  David Pierre Nov 15 '12 at 16:55
add comment

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()

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.