It gets a bit more complicated than that, but those examples should illustrate the basic behavior.

exit shouldn't be in there anyway though. Would you actually want script execution to completely stop? Typically exit should only be used if there is no way to continue the program execution without compromising security or stability.

Imagine if instead of throwing exceptions, you had only the exits there. This means that no error message would be displayed. Nothing would be logged. The code would just bail.

Imagine if someone was using your class based only on the API. They would have no idea why the code exited.

Typically low level flow control like exiting or redirecting to a different page, so on, should be handled in dedicated "controller" code and not in classes that have responsibilities unrelated to that.

Throwing an exception is the right thing to do here. It allows the code that called rule to decide how to handle it. rule should only responsible for saying "Hey, ummm, something wen't horribly wrong," not deciding how to handle it. (In fact, an uncaught exception is basically an exit with debugging information included :p. You do have to be careful with uncaught exceptions though as you need to make sure they don't get displayed to end clients. Some exceptions, like PDOException, may contain sensitive data like database credentials.)

Design Issues

Your class does way too much. This is just using a class as a collection of functions. Each validation can be seen as a "responsibility" and thus your class completely violates the single responsibility principle.

Imagine that in the future you want to add a new validation. How can you do this? Well, you have two options, both of which are unpleasant:

Edit the source code. What if many people are using your class though, or you're using it across 10 applications? Suddenly you have a lot of version management going on. Since you control the class, you could just commit it all to git which would alleviate the synchronization, but what about end-consumers of your code? This isn't a viable option for them.

Extend the class. This is also a bad option. You would have to change all of the object types in your code to be a MyValidation instead of Validation. Depending on how widespread this was, this could be a huge chore.

Or, you have a third option. Scrap your current design and abstract each validation into its own class. You could have a very simple interface:

This looks very laborious at first, but it gives you a certain degree of flexibility you didn't have before. Consider how you add a new validator now. You just create a new class completely unrelated to any of the other ones. (Technically it would implement Validator, and perhaps extend an abstract base class. What I mean though is that it doesn't change any of the other classes.)

In this system, consumption of your code is much simpler too. How does someone that doesn't have write access to your git repository create their own validator? They just implement validator in their own namespace rather than in yours. (Oh, side note, you should probably be using namespaces.)

For simplicity, you could make a "composite" Validator that just ties a bunch of validators together:

$range = new RangeValidator(1, 100);
$even = new EvenValidator();
$comp = new CompositeValidator(array($range, $even));
$comp->validate(5); //Returns an array of one message: 5 isn't odd
$comp->validate(2); //Returns an empty array
$comp->validate(222); //Returns an array of one message: 222 out of range
$comp->validate(223); //Two messages: 223 out of range and odd

This is some pretty clumsy usage here. With more than 5 seconds of thought though, an interface could be designed that allowed for very robust functionality while leaving the code highly maintainable and extensible.

You could even take it a step farther than a composite validator and make a "Form" class that has "Element"s which each have zero or more "Validator"s. (The approach take by Zend Framework, and I'm sure others).

If you're familiar with any PHP frameworks, you might note that all of that above looks eerily familiar. In particular, that's a modified version of the Zend_Validate interface from the Zend Framework.

I would suggest you pick a framework, go through the code, figure out how they designed things, and then ask yourself why they designed them that way: what the alternatives could have been and why they picked the path they chose. If you do look at frameworks for design, make sure you look at more than one. Though most of the major PHP frameworks/libraries are very well written overall, all of them have at least one area where they far from shine.

Also, if you haven't already, read everything you can find about SOLID.

Thanks man! Yeah, I wanted something that you could instansiate for each form you're validating, which is why I didn't use static methods. Good suggestion though! Appreciate the compliments :)
–
PrashNov 19 '12 at 22:18

Please don't use static methods as was suggested to you by max.haredoom. Static methods cause tight coupling in the code that calls them. See the following answers for reasons to avoid static: Why, oh why and what to do?

Properties

Remove the underscore from $_errors. See this answer for the reason that underscore should not be used for class properties.

errorText should be defined as a property of the class and its accessibility should be defined.

Constructor

require_once in the constructor is a bad idea. This class cannot stand alone as a unit and it is completely broken with more than one object. Pass in the required information as arguments to the constructor.

rule method

The name of this method does not explain what it is doing.

Pass $inputRules as an array (and typehint for it). I can't see the reason for mashing together rules as a string when they are a list of rules. The array type represents lists very well, so I would use it and avoid having to explode the string on |.

public function rule($value, $name, Array $inputRules)

Your code for throwing an exception is repeated. Calculate the variables in the if / else and then check only once.

The variables should be renamed to something more meaningful. $rule should be $validationMethod and $call should be $validationArguments.

Use is_callable rather than method_exists it ensures that you will be able to call it (which is what you really want).

exec method

This does not exec anything (thankfully).

Perhaps it should be named validate, but I would have expected it to return true when there weren't any errors?

Design

I agree with Corbin. A validation interface is a great way to do this sort of thing. I think the key part of a validation class should be returning whether the data is valid or not. Your class does not make this a focus. Personally I like a simple interface that checks the validity and returns a boolean of whether the data is valid or not. The other thing that is needed is a method to return the errors from the last validation.