3

If a method takes an object of class A as parameter and analyzes its properties somehow, performs calculations etc. to verify this object, is it okay for class A to have a boolean valid field? After the method of class B completes verification of this object it sets the boolean flag accordingly.

The verification process needs some additional info based on objects of classes other than A, so it's impossible to put a verify method in A - the object can't just verify (or validate) itself, just like it doesn't make much sense for a Car class to have a repair method. But it make sense to have a workingFine flag I guess in a Car. Then is it better to store it as a flag in A? Or should I do something else?

3 Answers 3

3

In your case, the object itself is not "valid". It is "valid in some context". This context is defined as "additional info" you described. As such, it should be obvious in which context the object is valid.

I would say putting IsValid flag in the object doesn't make much sense in this case. Only case it would make sense if the context is closely tied to the object itself. In which case, the object itself can have reference to this context.

Otherwise, I would create special "validation" object, that would bind together the information about what object, what context and if it is valid.

1
  • In my case the validity of the object is closely tied to its class. However it doesn't make much sense to store all information necessary for verification in this class. So yeah I guess its okay to make it a field. Commented Apr 8, 2016 at 21:04
1

Ideally, you shouldn't need to store this information explicitly at all. It should follow implicitly from the control flow of your program.

;; Very easy to read

ROUTINE ProcessItem(item)
BEGIN
    IF IsValidItem(item) THEN
        ProcessValidItem(item)
    ELSE
        ProcessInvalidItem(item)
    FI
END

ROUTINE ProcessValidItem(item)
BEGIN
    DoSomething(item)
    DoAnotherThing(item)
    DoThisThing(item)
END

ROUTINE ProcessInvalidItem(item)
BEGIN
    DoSomethingElse(item)
    DoAnotherThing(item)
END

Inside the subroutines ProcessValidItem and ProcessInvalidItem, I don't need a flag that tells me whether an item is valid. If the structure of your program implies the validity of an item, you also cannot forget checking it. Validity should be checked at exactly one point, if possible. Instead of combining the treatment of valid and invalid items in the same function and deciding repetitively what to do depending on a “valid” flag, separate the control flow into two distinct, straight-forward code paths and factor the parts that are common to the treatment of both, valid and invalid items, out into subroutines. I find the code shown above much cleaner than the following example.

;; Not so easy to read

ROUTINE ProcessItem(item)
BEGIN
    valid ← IsValidItem(item)
    IF valid THEN
        DoSomething(item)
    ELSE
        DoSomethingElse(item)
    FI
    DoAnotherThing(item)
    IF valid THEN
        DoThisThing(item)
    FI
END

If you do have to store the validity on a per-item base, this flag should be owned by whomever determines the validity.

If an object can validate itself, it should have a method that tells whether it is valid. Whether it validates itself each time this method is called or caches it in a private field is an implementation detail. It would clearly break encapsulation to let code outside that class mess with the value of that field.

On the other hand, if the object is validated by someone else, then this somebody is responsible for keeping the information – outside the object. Giving the object access to its externally determined validation status would be just as wrong as giving somebody else access to the object's internal state. A simple pair structure could be used to attach such information to an object – and remove it again when no longer needed. In an object-oriented setup, the flag could be replaced by polymorphism to save one word of storage for the cost of a virtual method call.

                    +-------------------------+
                    |    ValidatedItem<T>     |
                    +-------------------------+
                    | - item : T              |
                    +-------------------------+
                    | + getItem() : T         |
                    | + isValid() : Boolean   |
                    +-------------------------+
                                 ^
                                 |
             +-------------------+-------------------+
             |                                       |
+-------------------------+             +-------------------------+
|      ValidItem<T>       |             |     InvalidItem<T>      |
+-------------------------+             +-------------------------+
| + isValid() : Boolean   |             | + isValid() : Boolean   |
+-------------------------+             +-------------------------+
4
  • If a Person class has a salary attribute, it ususally has a setter for this attribute, even though the logic for determining the new salary is not a part of this class. Similarly here, the logic for maintaining the object I'm trying to validate is not a part of this class. My point is that in my case such a solution would be an extreme overcomplication. Commented Apr 9, 2016 at 8:34
  • The ValidatedItem class objects are created by the class responsible for validating the objects, right? One more thing - the getItem method should return a clone of the object, because if it returned the object itself I could call item's methods which could potentially invalidate it. Then the ValidatedItem object would no longer be valid. Commented Apr 9, 2016 at 10:50
  • But I don't think ValidatedItem class should contain the logic for validating it. In this case, the only way isValid method might work is by rerurning a boolean flag, which should be a private field of the ValidateItem class, right? Commented Apr 9, 2016 at 12:07
  • @user4205580 If mutability is a concern, then yes, you could consider returning a clone of the object from getItem, though it might be costly. isValid doesn't validate, nor does it return a boolean flag. In ValidItem, isValid is implemented as return true;, in InvalidItem, it is implemented as return false;. In your validator, you instantiate the appropriate sub-type and return it polymorphically as a ValidatedItem. Commented Apr 11, 2016 at 17:35
0

One option would be to adapt the pattern found in C#'s IValidatableObject interface. As you can see it takes a ValidationContext object as a parameter. The object can then use that context to validate itself for the specific purpose/context that it is being validated against.

I don't particularly like this pattern1 but it is a way for your objects to validate themselves in a context specific manner. In this case I think it is much better though than your current solution of a generic boolean valid field/property that is set by another class based on unknown validation context.

An alternative to some sort of interface that lets you specify the context for validation would be to wrap the object. That would give you the benefits of setting the property/field without breaking encapsulation or requiring a sub class for each object. You don't mention the language you're using but something like the following would work in C#:

public class Validated<T>
{
  public Validated<T>(bool isValid, T validatedObject)
  {
     IsValid = isValid;
     ValidatedObject = validatedObject;
  }
  public bool IsValid { get; }
  public T ValidatedObject { get; }
}

You could add another property to the class above to specify under what context the class is considered valid if there is any worry that the context might be ambiguous at some period in the object's lifetime.


1: General blog post I wrote about validation that briefly mentions this pattern.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.