How to handle incorrect class usage within legacy code (Exceptions?)

[dennis-fedco]

Imagine a huge legacy code base.  Spaghetti code everywhere.  Nobody knows all of the code, or if the code behaves.

 

Now, imagine I wrote a class, and I know that the class behaves, if it is given proper input values, and is properly instantiated and proper setter methods are used.

But, I have no guarantee that my class will be called correctly.  I cannot assume that any error checking is being done elsewhere.  How then do I proceed?

 

Example:

class Device
{
    private $modelNumber,$particleCount;

    public function __construct($modelNumber) {
        $this->modelNumber=$modelNumber;
    }

    public function setParticleCount($particleCount) {
        $this->particleCount= $particleCount;
    }

    public function getComputedData() {
        //...
        return array(..);
    }
}


//calling class/method:
$var=new Device(123);       //Problem #1 - if incorrect model number is given, my code will not create a "device" internally, and getComputedData() will fail.
$var->setParticleCount(50); //Problem #2.  If this method is not called, or called with a number outside of specs, getComputedData() will fail.
$var->getComputedData();    //gets the payload if model number and particle count were called correctly.

Some ideas I have:

• Idea 1:  put some detection code in my class and print out or log a message "Incorrect data given" or the like upon detection.  Bad because purpose of my class is to compute data, not alert users of errors
• Idea 2:  throw an exception. I think it is expensive in terms of computation but may be worth it
• Idea 3:  ignore incorrect data silently, i.e. not try detect it.  This may return incorrect data back to the caller, or fail in other unspecified ways.

I think Exceptions is best way so far but I wanted to check if there is a better way altogether to handle this, maybe via design or some other way or method.  If so, what is it?

Edited February 20, 2014 by dennis-fedco

[dennis-fedco]

ah,  well first thing I realized that particle count is actually required for the class to operate properly, so then I might as well put it into the constructor.  Phew!

$var = new Device(123, 50);
$var->getComputedData();

So That takes care of the problem of "someone forgetting to call the particle setter method".  I still have the problem of supplying incorrect/non-existent data values, such as model number and particle count.

[dennis-fedco]

To answer myself.. I think it is responsibility of the caller to ensure that calling my class succeeds.  I was trying to have my class to let the caller know it failed.

 

so instead of

$var = new Device($x, $y); //have it fail somehow if $x or $y are bad

I need to do

if (valid($x) && valid($y))
    $var = new Device($x, $y); 
else
    blow_up();

Still not the best way, as here I spread my Device logic onto some function called valid();   I do not like that solution yet.  Perhaps I can have a DeviceChecker() class that can be used to verify Device data before creating the device.

$deviceChecker = new DeviceChecker("device_name", $x, $y);

if ($deviceChecker->isValid())
    $var = new Device($x, $y);
else
    blow_up();

Well, now I have some class that I need to update that has part of my device logic.  I suppose I can move it into Device class, and call Device::isValid(), but that requires the caller to know that they are to call this function.  Bleh.

Edited February 27, 2014 by dennis-fedco

[ignace]

Simply do the checks inside Device and throw an exception when an invalid value is encountered.

 

try {
  $device = new Device($foo, $bar);
} catch (InvalidArgumentException $e) {
  echo 'I see what you did there!';
}

It's the responsibility of the programmer to write the class that the invariants are intact ALWAYS. Edited February 27, 2014 by ignace