views:

78

answers:

3

I know that this type of question has been asked over and over again, however, I have yet to find a definitive answer for the problem I am looking into.

From all the content on exception handling that I have read it appears that the general concensus is that exceptions should only be used for exceptional circumstances. I've also read in many places that one should use, where possible, return values to indicate problems or failures (such as login failure, validation failure of some sort). My problem is, when using these return values, how does one communicate the contextual information of the problem? With exceptions, one can add the contextual information to the exception and allow that to bubble up. Let me try and use a code example to explain:

Let's say we have a basic abstract class (I've left out some of the details) which represents some kind of format definition for a String. This class essentially dictates how the format of a given string should be.

public abstract class ADataEntryDefinition
{
    public boolean isValid(String data);
}

let's say I extend this to perform some security validation on the string:

public class SecureDataEntryDefinition extends ADataEntryDefinition
{
    public boolean isValid(String data)
    {
        //do some security checks on the format of the data
    }        
}

The validate method will take in a String and return true if the string matches the data definition defined by the class.

Moving along, let's say I have a class which manages several of these data definitions, and this class' responsibility is to validate each entry in a comma separated String against one of the data definitions it maintains.

public class DataSetDefinitions
{
    private List<ADataEntryDefinition> dataDefinitions = ...

    public boolean isValid(String dataValues)
    {
        //obtain each string in dataValues delimited by a ',' into String[]
        //called dataEntryValues

        int i=0;
        for (ADataEntryDefinition dataEntry : dataDefinitions)
        {
            if (!dataEntry.isValid(dataEntryValues[i++])
            {
                return false;
            }
        }
        return true;           
    }        
}

Now, to me these methods seem way to general to throw exceptions in the event of invalid data (for one, invalid data may be expected in some cases). In this case, I like the approach of returning true/false to indicate validation failure and subsequently allowing the caller to judge how serious it is. So the caller does the following:

boolean success = false;
success = dataSetDefinitions.isValid(someString);

Suppose a specific caller like the above deems the failed validation to be critical, and hence, must subsequently throw an exception to prevent processing from continuing; where should it obtain the contextual information it needs to convey the problem... how should it know that 2 layers (calls) down the validation actually failed due to security problems in the SecureDataEntryDefinition class (or any other subclass for that matter).

I guess I could add a method like so:

public class DataSetDefinitions
{
    private List<ADataEntryDefinition> dataDefinitions = ...

    public boolean isValid(String dataValues)
    {
        ....
    }

    public String getValidationErrorMsg() {...}
}

which would return the error message of the last failed validation. Then, the following could be done by the caller upon failed validation:

success = dataSetDefinitions.isValid(someString);
if (!success)
    throw new SomeException(dataSetDefinitions.getValidationErrorMsg());

But to me this just seems like having the class (DataSetDefinitions in this case) know or maintain state about the previous validation which it shouldn't. Taking into account that this class may perform validation of several different, independent strings, it seems wrong having it maintain state about the validation of any given one of them.

I guess this question is essentially asking how one designs methods to be general - not taking the law into their own hands by throwing exceptions unnecessarily, but allowing callers to decide on the severity - but still allowing the callers to obtain detailed contextual information in the event that the caller needs to communicate the problem. Is there a better way of doing the above?

Apologies if this was very long-winded :/ Any responses will be appreciated.

Ciao.

+1  A: 

You could return a user defined class instead of a simple bool in order to provide more contextual information.

It would be something similar to the strategy used with events. We have a EventArgs class from which other classes derive in order to provide more contextual information for a given type of event.

João Angelo
+1  A: 

Don't return a bool. Return a class that encapsulates the success/failure state, plus the associated information. That way, you can do something like:

DataEntryStatus status = isValid(...);

if (!status.isValid()) {
   throw status.generateStatusException();
}

and the status object itself generates the appropriate exception, thus maintaining encapsulation.

Brian Agnew
A: 

The way i solve it most of the time is defining several class constants and return these. Then in the business logic of my controllers i would just check against these values statically.

<?php
class Test
{
   const SUCCESS = 1000;
   const EMAIL_FAIL = 2001;
   const SAVE_FAIL  = 2002;

   ...

   public function save($value)
   {
      if (!$this->writetodb($value)
         return self::SAVE_FAIL;
      elseif(!$this->sendMailToAdmin())
         return self::EMAIL_FAIL;
      else
         return self::SUCCESS;
   }
}




$test = new Test();
$result = $test->save('my value');
switch ($result) {
   case Test::SUCCESS:
      echo 'Yay!';
      break;
   case Test::SAVE_FAIL:
      echo 'Error saving!';
      break;
   case Test::EMAIL_FAIL:
      echo 'Error sending email!';
      break;
}
ChrisR