[adn258]

Like so so you don't have to type this over and over again for example you have a function

<?php

function BeginQuery()
{
  $dbh = new PDO('mysql:dbname=test;host=localhost','','');
  return $dbh;
}


?>

Then later on you could use something liek

$q = BeginQuery();
$query = "INSERT INTO testing(name)VALUES('hi');";
$q->exec($query);

Is this acceptable to do this? Is there anything bad about this?

[andrewsw]

As this stands your function is doing very little and, importantly, doesn't handle errors. IMO functions of one or two simple lines generally add unnecessary function calls - unless you consider them useful to help organise your thoughts/logic.

I would have the function trap and re-throw any error which your main code would need to re-catch (although, I actually implement a generic error handler) - then your function becomes more useful.

[Dormilich]

adn258, on 23 February 2013 - 11:06 AM, said:

Is this acceptable to do this? Is there anything bad about this?

that is called a "Factory" (though in a very simple way).

the only disadvantage I see is that there is no hint, what object BeginQuery() returns. i.e. unless you know that BeginQuery()’s object has an exec() method, it is very error prone to call that method (e.g. PDO has an exec() method, but mysqli does not. so by switching from one to the other, you would invalidate your code). To solve that problem, one usually uses Interfaces to define the guaranteed methods.

andrewsw, on 23 February 2013 - 11:23 AM, said:

I would have the function trap and re-throw any error which your main code would need to re-catch

why? if the function throws an exception anyways, then you can let through the original one. besides that re-throwing an exception blurs the stack trace (the PDOException would tell you, that it is a PDO problem, a custom exception would tell you, that it is a BeginQuery() problem, so you lose information).

as a rule of thumb, only catch exceptions when you can handle their problem.

[andrewsw]

Quote

why? if the function throws an exception anyways, then you can let through the original one. besides that re-throwing an exception blurs the stack trace (the PDOException would tell you, that it is a PDO problem, a custom exception would tell you, that it is a BeginQuery() problem, so you lose information).

as a rule of thumb, only catch exceptions when you can handle their problem.

Yes, you are right, sorry; new PDO already throws the error. It's early in the morning!

[Dormilich]

andrewsw, on 23 February 2013 - 11:36 AM, said:

It's early in the morning!

I’m already thinking about lunch …

[adn258]

Thanks guys I was just speaking in generalizations here. Obviously it would be better to have a class and an interface or something to make the code more straight forward.

I'm confused though as to why you wouldn't catch the exception with something like

try
 {
 $dbh = new PDO('');
 return $dbh;
 }
 catch (PDOException $e) {
 echo 'Connection failed: ' . $e->getMessage();
 exit();
 }

Correct me if I'm wrong but PDO and MYSQLI connections that fail and aren't handled like this can actually output a bunch of security information to the page viewer INCLUDING DATABASE PASSWORDS? Someone told me that once though I don't know if it's true?

[Dormilich]

it is true that if the failed code is not handled properly, the resulting error output can display your database credentials.

the point above with catching the exception was not whether to catch the exception at all, but where to catch the exception.

to the above code. while PDO throws an exception on failed instantiation, mysqli does not (i.e. you have to explicitly check for errors in mysqli).
when you catch an exception you also should not print the error message to the screen, but log the message somewhere save or mail it to the admin.

This post has been edited by Dormilich: 24 February 2013 - 01:35 AM

[adn258]

Dormilich, on 24 February 2013 - 01:30 AM, said:

it is true that if the failed code is not handled properly, the resulting error output can display your database credentials.

the point above with catching the exception was not whether to catch the exception at all, but where to catch the exception.

to the above code. while PDO throws an exception on failed instantiation, mysqli does not (i.e. you have to explicitly check for errors in mysqli).
when you catch an exception you also should not print the error message to the screen, but log the message somewhere save or mail it to the admin.

So what you are saying is that it's not necessary to use try and catch like I just did when using PDO?

[Dormilich]

no, I didn’t say that.

I said that echoing an exception’s errormessage to the screen can reveal the database credentials.

I said that error messages should be sent to the admin.

I said that try…catch using mysqli is pointless (unless you manually convert errors to exceptions).

I said that you should catch an exception where you can handle its problem.
for your example, that would not be in the beginQuery() function, because the function cannot handle a failed connection.

[adn258]

Dormilich, on 24 February 2013 - 03:56 AM, said:

no, I didn’t say that.

I said that echoing an exception’s errormessage to the screen can reveal the database credentials.

I said that error messages should be sent to the admin.

I said that try…catch using mysqli is pointless (unless you manually convert errors to exceptions).

I said that you should catch an exception where you can handle its problem.
for your example, that would not be in the beginQuery() function, because the function cannot handle a failed connection.

Ok that makes sense now. Thanks friend!! So it would be a waste though to use try and catch AROUND EVERY PDO QUERYthen correct?

You could again use try and catch just usinga simple query?

This post has been edited by adn258: 24 February 2013 - 01:09 PM

[Dormilich]

adn258, on 24 February 2013 - 08:58 PM, said:

So it would be a waste though to use try and catch AROUND EVERY PDO QUERYthen correct?

correct.

adn258, on 24 February 2013 - 08:58 PM, said:

You could again use try and catch just usinga simple query?

that depends on the query, its dependent code and the kind of exception it emits. (and of course you cannot start a query if you have no connection.)

for example if you have a SELECT query whose result is printed in a table/list, it doesn’t suffice to enclose just the query. you have dependent code that relies on the query being successful. if the query errors out, the table cannot be displayed, hence you need to enclose all the display code in the try…catch as well so that in case of an error the dependent code is skipped (and you can display a default (e.g. an explanation that something went wrong) instead).

other example, you have an INSERT. often there is no following code that depends on the insert, so here it would suffice to just enclose the query.

This post has been edited by Dormilich: 24 February 2013 - 01:41 PM