Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Your pdoDatabase class shouldn't be a singletonsingleton.

Your pdoDatabase class shouldn't be a singleton.

Your pdoDatabase class shouldn't be a singleton.

added 1692 characters in body
Source Link
Corbin
  • 10.6k
  • 2
  • 31
  • 51

Exception Catching

It should be extremely rare for an exception to actually happen with your Mobile.

Consider the sources of exceptions: connection drops, invalid syntax, unknown column/table/etc, so on. Basically there's different layers of exceptions. There's the connection oriented ones, and the SQL oriented ones. The SQL oriented ones should never happen. By the time your code goes into production, you should be certain that none of your queries have invalid syntax, reference non-existing entities, so on.

This leaves connection problems as the only source of exceptions (or perhaps MySQL related hiccups, though those should be pretty rare).

Because of this rarity, I would be tempted to basically ignore any exceptions.

Even if you did handle the exceptions, what are you going to do with them?

try {
    $affs = $mobile->findPhoneAffiliate(5);
} catch (SomeExceptionClass $ex) {
    //There's not really a good way to recover from this, so you might as well
    //show some kind of "Oops, something broke." page.
    //You could then log the exception behind the scenes and make sure the end-user
    //sees nothing sensitive.
}

What I would do in this situation is just let the exception bubble up to the very top. I would have a giant try/catch at the top layer though that intercepts any uncaught exceptions and renders some kind of error page.

How you do this will highly depend on the structure of your website, but assuming you're doing something MVC-ish, it should be fairly simple. In the catch block, just kill the current dispatch, and instead dispatch a request for the exception page.


Exception Catching

It should be extremely rare for an exception to actually happen with your Mobile.

Consider the sources of exceptions: connection drops, invalid syntax, unknown column/table/etc, so on. Basically there's different layers of exceptions. There's the connection oriented ones, and the SQL oriented ones. The SQL oriented ones should never happen. By the time your code goes into production, you should be certain that none of your queries have invalid syntax, reference non-existing entities, so on.

This leaves connection problems as the only source of exceptions (or perhaps MySQL related hiccups, though those should be pretty rare).

Because of this rarity, I would be tempted to basically ignore any exceptions.

Even if you did handle the exceptions, what are you going to do with them?

try {
    $affs = $mobile->findPhoneAffiliate(5);
} catch (SomeExceptionClass $ex) {
    //There's not really a good way to recover from this, so you might as well
    //show some kind of "Oops, something broke." page.
    //You could then log the exception behind the scenes and make sure the end-user
    //sees nothing sensitive.
}

What I would do in this situation is just let the exception bubble up to the very top. I would have a giant try/catch at the top layer though that intercepts any uncaught exceptions and renders some kind of error page.

How you do this will highly depend on the structure of your website, but assuming you're doing something MVC-ish, it should be fairly simple. In the catch block, just kill the current dispatch, and instead dispatch a request for the exception page.

Source Link
Corbin
  • 10.6k
  • 2
  • 31
  • 51

I'll probably revisit this later, but for now:


The exception catching should be done at either the model or model-using layer. I would probably let the exceptions bubble up through the model unless you want the model to be fairly high level.

For the sake of abstracting the DB layer away from the model layer, I might consider wrapping the PDOException in some kind of model exception(s). Exception wrapping is typically an anti-pattern, but it would keep your consuming code from needing to know that the models are all dependent on PDO. (Thus allowing you to change PDO to something else if you ever wanted to -- basically it's a weird instance of separation of concerns and hiding implementation details.)

I'm starting to ramble, but what I'm trying to convey is that your consuming code needs to know something went wrong. If you handle the exception in your DB class, you model has no (pleasant) way to know that something went wrong. If you handle the exception in your model, the level above the model has no (pleasant) way to know something went wrong. Basically your consuming code needs to know something happened, and for that to be the case, you need exceptions to come out of the model. (An alternative would of course be returning a Boolean or something along those lines. That tends to not fit the OO paradigm very well though, and PDO sometimes uses Boolean false when something isn't an error.)


Your pdoDatabase class shouldn't be a singleton.


$this->db = pdoDatabase::getInstance(); you pass the pdoDatabase instance as a parameter instead of coupling your code to this static method.

Dependency Injection


There's a few other things that stuck out to me, but I'm out of time now ;(. Will come back later.