2
\$\begingroup\$

I am still getting the hang of Exception Handling in PHP; Here is the scenario where I am using the convention: There are times where I have a SQL query that I need to turn into an object, so I wrote this utility class/method.

Class A {
    public static function getThatOneInstance(){
        $sql = "SELECT id FROM table WHERE condition=met LIMIT 1";
        $O = UtilsMysql::fetchIdIntoInstance($sql, __CLASS__, new Exception('Not Found!?'));
        return $O;
    }
}

Class UtilsMysql {
    public static function fetchIdIntoInstance($sql, $Class, Exception $NotFoundE = NULL){
        $O = NULL;
        $r = mysql_query($sql);
        if (!$r) throw new ExceptionMySQL($sql);
        if (mysql_num_rows($r) > 0) {
            list($id) = mysql_fetch_row($r);
            $O = new $Class($id);
        } elseif (!is_null($NotFoundE)) {
            throw $NotFoundE;
        }
        return $O;
    }
}

There are times, though, where if there is no result returned from the query it represents an Exceptional Circumstance, and so an Exception needs to be thrown. Because this is not uncommon, rather than always testing the returned value from the utility method, I have started passing in an Exception, that will be thrown if no results are found. This way, I have a contract with the function, if I pass in an exception, I can rely on the fact that it returns an Object (don't need to test it).

Is it acceptable to pass an Exception into a function that then may or may not throw it?

\$\endgroup\$
1
\$\begingroup\$

As Winston said, use full names for your variables. Also keep the case consistent - then when you write $Object instead of $object it will be more obvious.

Static should almost never be used if you are writing object oriented code. You basically have namespaced functions with this code. Rather than write a static utils class for SQL I would write an SQL class (or use an existing one). If you want objects of a class to be created PDO can do this for you.

As far as the exception goes:

No, you shouldn't do that. Your code creates an object that it doesn't need to every time the function is called. At the worst you would have passed in the exception string and instantiated with new Exception in fetchIdIntoInstance.

Main Recommendation

You are better of dealing with an exception after it has happened at a level where you know why it happened.

Keep your lower level functions simple: If something strange happens throw an exception. In your case I would assume that your sql should work in fetchIdIntoInstance and throw an exception if you can't make the new instance.

At a higher level database operations will have a meaning. You will be doing something specific that is easily understood. Maybe you will be adding a record to a table or reading an object that you expect to be in the database. At this point you can handle the exception and write meaningful information to a log file or something similar.

You could catch exceptions in getThatOneInstance but it does not seem like the right place. I think you should catch it higher up where you know what to do with it.

\$\endgroup\$
0
\$\begingroup\$

Its kinda odd.

I'd suggest having a second function that throws exceptions when the row is not returned.

I'd also strongly recommend you stop using variables like $O and $r, they make your code much harder to read.

\$\endgroup\$
  • \$\begingroup\$ an entirely new function? in which class? And are you saying you would prefer $Object and $result, over $O and $r? \$\endgroup\$ – Shad Dec 12 '11 at 20:04
  • \$\begingroup\$ @Shad, yes I'd prefer spelled out words. I'd know what they were for as soon as I read them. I wouldn't put either function in a class, but if you insist, I'd put it the same class as fetchIdIntoInstance. \$\endgroup\$ – Winston Ewert Dec 12 '11 at 23:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.