views:

57

answers:

2

I have a data processor class which can only perform its primary function once all its member variables have been assigned a value:

class {
    public $firstName;
    public $lastName;
    public $ssn;
    public $accessKey;

    public function __construct($data = null) {
        if (is_array($data)) {
            // Assign the value of any fields in $data to
            // the corresponding member var
        }
    }

    public processData() {
        // *** CHECK IF ALL PROPERTIES HAVE VALUES ***
        foreach ($this as $p=>$val) {
            if ($val === null) {
                return false;
            }
        }

        doStuff();
    }
}

Is there a more efficient or elegant way to verify that all properties have a value? It feels kind of PHugly to do it this way.

+1  A: 

Well i would encapsulate the checks in a protected method like _isValid() and then jsut do

public function process()
{
  if($this->_isValid())
  {
     $this->doStuff();
     return true;
  }

  // otherewise throw an exception or otherwise alter control flow return values
}

Another thing to make tha actuall check more elegant would be to add a variable for _requiredValues and one for _values and have them both be arrays - instead of using individual member variables... this way you can check them wholesale using an array comparison function if you prefer.

If you want easy access to individual values you could just add a getter like `public

function getValue($value) 
{ 
    return isset($this->_values[$value]) 
       ? $this->_values[$value] 
       : null; 
}
prodigitalson
I'm actually already wrapping the check in a separate method, just omitted that for brevity. I'm not opposed to the idea of using an array to store required values, but I'd like to see if there are any other suggestions first.
Brian Lacy
+1  A: 

You could put the class members into an array so you can iterate over them without including all other class members, example:

<?php

class Test
{

    public $options = array
    (
        'firstname' => NULL,
        'lastname' => NULL,
        'ssn' => NULL,
        'accesskey' => NULL,
    );

    public function __set($key, $val)
    {
        if (empty($val) === FALSE AND array_key_exists($key, $this->options))
        {
            $this->options[$key] = $val;
        }
        else
        {
           // Throw an exception
           throw new Exception('Empty value');
        }

        return;
    }

    public processData()
    {
        doStuff();
    }            
}

There's an error in your code, you forgot the "function" syntax on "processData".

I've also created a __set method which throws an error when you set an empty value. For example

<?php

$test = new Test;

try
{
    // Throws an error
    $test->firstname = NULL;
} 
catch(Exception $e)
{ 
   var_dump($e);
}

try
{
    // Works fine
    $test->firstname = 'Brian';
} 
catch(Exception $e)
{ 
   var_dump($e);
}
The Pixel Developer