Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have a class that I am using to initialize a form for editing. Once the form is submitted, data validation is done through the setters of my class. Good or bad input will be set in order to re-display them in the form (is this bad?). Everything works fine, but I am not too happy with how I am storing and displaying the error messages.

Any suggestions on how I can improve?


class Course {
    protected $_code;
    protected $_name;
    protected $_errors = array();

    public function __constructor($id) {
        if (is_int($id)) {

    public function getCode() { return $this->_code; }

    public function setCode($value) { 
        $this->_code = $value;
        if (empty($value)) {
            $this->setError('code', 'Enter a code.');
        } else if (strlen($value) > 10) {
            $this->setError('code', 'The code you provided must be less than 10 characters.');

    public function getName() { return $this->_name; }

    public function setName($value) { 
        $this->_name = $value;
        if (empty($value)) {
            $this->setError('name', 'Enter a name.');
        } else if (strlen($value) > 50) {
            $this->setError('name', 'The name you provided must be less than 50 characters.');

    public function getError($name) {
        return isset($this->_errors[$name]) ? $this->_errors[$name] : '';

    public function setError($name, $value) {
        $this->_errors[$name] = $value;

    public function hasErrors() {
        return !empty($this->_errors);

    public function _load($id) {
        // load record from database

    public function save() {
        if (!this->hasErrors()) {
            // save record to database

I have looked around and many have suggest to use exceptions. However, I don't quite get how I can use them because wouldn't throwing an exception in one setter cause the subsequent setters from not being called.

Here is how I am currently using my class:


$filter = array(

$c = filter_input(INPUT_GET, 'c', FILTER_VALIDATE_INT);
$course = new Course($c);

$inputs = filter_input_array(INPUT_POST, $filter);
if ($inputs['hidSubmit']) {

require_once 'header_admin.php';

<form id="editForm" name="editForm" action="" method="post">
<legend>Edit Course</legend>
    <input type="text" name="code" value="<?php echo $course->getCode(); ?>" maxlength="10" size="8" />
    <span class="error"><?php echo $course->getError('code'); ?></span>
    <input type="text" name="name" value="<?php echo $course->getName(); ?>" maxlength="45" size="25" />
    <span class="error"><?php echo $course->getError('name'); ?></span>
    <input type="submit" value="Save" />
    <input type="hidden" name="hidSubmit" value="1" />

require_once 'footer_admin.php';
share|improve this question
check out how other frameworks use forms for validation : – Rob Apodaca Feb 17 '13 at 15:57
Oh. I'd have to rewrite many things but thanks though. I'll keep that in mind next time. – Mikey Feb 17 '13 at 16:33
Post rolled back to undo answer invalidation. – Jamal Dec 29 '14 at 1:19
up vote 3 down vote accepted

The big disadvantage of putting the validation to the setter is that you are never able to set invalid values. In most cases this is fine, but in some scenarios a valid user input differs from a more general valid field content. In addition the validation is also executed to internal setter uses where know that they are valid.

That said, I recommend setters to be only setters and not more.

The validation itself can be placed in a separate method or even in a separate class (depending on the complexity).

If you only want to display one error message, throwing an exception within the validation should be fine.

If you want to collect multiple error messages, you might want to build a generic validation class:

$valid=new Validation();
$valid->assertNotEmpty($name,"name","Insert a name please");
if ($valid->hasErrors()) ...

You could extend this to a CourseValidation if you need it at multiple locations.

$valid=new CourseValidation($code,$name);
share|improve this answer
Thank you, it works but I am not sure if I am understanding your suggestion correctly. I have updated my code to show what I did. Is that what you mean? Any more improvements that can be made? – Mikey Feb 17 '13 at 16:30
You should also consider Robs comments. Creating a Form class will help you to get rid of the duplication in your HTML. – mnhg Feb 17 '13 at 17:59

Your Answer


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.