tags:

views:

172

answers:

4

I'm writing a web application (PHP) for my friend and have decided to use my limited OOP training from Java.

My question is what is the best way to note in my class/application that specific critical things failed without actually breaking my page.

Currently my problem is I have an Object "SummerCamper" which takes a camper_id as it's argument to load all of the necessary data into the object from the database. Say someone specifies a camper_id in the querystring that does not exist, I pass it to my objects constructor and the load fails. Currently I don't see a way for me to just return false from the constructor.

I have read I could possibly do this with Exceptions, throwing an exception if no records are found in the database or if some sort of validation fails on input of the camper_id from the application etc.

However, I have not really found a great way to alert my program that the Object Load has failed. I tried returning false from within the CATCH but the Object still persists in my php page. I do understand I could put a variable $is_valid = false if the load fails and then check the Object using a get method but I think there may be better ways.

What is the best way of achieving the essential termination of an object if a load fails? Should I load data into the object from outside the constructor? Is there some osrt of design pattern that I should look into?

Any help would be appreciated.

function __construct($camper_id){
        try{
            $query = "SELECT * FROM campers WHERE camper_id = $camper_id";
            $getResults = mysql_query($query);

            $records = mysql_num_rows($getResults);

            if ($records != 1) {
                throw new Exception('Camper ID not Found.');
            }

            while($row = mysql_fetch_array($getResults))
            {
                $this->camper_id = $row['camper_id'];
                $this->first_name = $row['first_name'];
                $this->last_name = $row['last_name'];
                $this->grade = $row['grade'];
                $this->camper_age = $row['camper_age'];
                $this->camper_gender = $row['gender'];
                $this->return_camper = $row['return_camper'];
            }
        }
        catch(Exception $e){
            return false;
        }



    }
A: 

Throwing an exception from the constructor is probably the right approach. You can catch this in an appropriate place, and take the necessary action (e.g. display an error page). Since you didn't show any code, it's not clear where you were catching your exception or why that didn't seem to work.

Matthew Flaschen
+1  A: 

A constructor in PHP will always return the object. This

public function __construct()
{
    return FALSE;
}

will not work. Throwing an Exception in the constructor

public function __construct($camperId)
{
    if($camperId === 1) {
        throw new Exception('ID 1 is not in database');
    }
}

would terminate script execution unless you catch it somewhere

try { 
    $camper = new SummerCamper(1);
} catch(Exception $e) {
    $camper = FALSE;
}

You could move the above code into a static method of SummerCamper to create instances of it instead of using the new keyword (which is common in Java I heard)

class SummerCamper
{
    protected function __construct($camperId)
    {
        if($camperId === 1) {
            throw new Exception('ID 1 is not in database');
        }
    }
    public static function create($camperId)
    {
        $camper = FALSE;
        try {
            $camper = new self($camperId);
        } catch(Exception $e) {
            // uncomment if you want PHP to raise a Notice about it
            // trigger_error($e->getMessage(), E_USER_NOTICE);
        }
        return $camper;
    }
}

This way you could do

$camper = SummerCamper:create(1);

and get FALSE in $camper when the $camper_id does not exist.

Another option would be to decouple the database access from the SummerCamper altogether. Basically, SummerCamper is an Entity that should only be concerned about SummerCamper things. If you give it knowledge how to persist itself, you are effectively creating an ActiveRecord or RowDataGateway. You could go with a DataMapper approach:

class SummerCamperMapper
{
    public function findById($id)
    {
        $camper = FALSE;
        $data = $this->dbAdapter->query('SELECT id, name FROM campers where ?', $id);
        if($data) {
            $camper = new SummerCamper($data);
        }
        return $camper;
    }
}

and your Entity

class SummerCamper
{
    protected $id;
    public function __construct(array $data)
    {
        $this->id = data['id'];
        // other assignments
    }
}

DataMapper is somewhat more complicated but it gives you decoupled code which is more maintainable and flexible in the end. Have a look around SO, there is a number of questions on these topics.

Gordon
+1  A: 
try {
    $camper = new SummerCamper($id);
    $camper->display();
} catch (NonexistentCamper $ex) {
    handleFailure($ex);
}
just somebody
Gordon, you want to follow this example to create and use your SummerCamper (this is the client code for your class), and REMOVE the try{}catch{} from your SummerCamper constructor. The way you have it written now, you're swallowing the exception before the client code can see it.
grossvogel
@grossvogel there is no try/catch in the constructor. I assume you are refering to the static function. There it is on purpose. As far as I understood the OP, he is not so much concerned about the actual exception than he is about simply not getting an instance. Also, if the try/catch block is outside the static method, he would have to repeat the try/catch whenever he needs to create a SummerCamper. That's not DRY.
Gordon
+1  A: 

To add to the others' answers, keep in mind that you can throw different types of exceptions from a single method and handle them each differently:

try {
    $camper = new SummerCamper($camper_id);
} catch (NoRecordsException $e) {
    // handle no records
} catch (InvalidDataException $e) {
    // handle invalid data
}
webbiedave
Yes. Good suggestion.
Gordon