current community

your communities

more stack exchange communities

Stack Exchange
tour help
careers 2.0
Take the 2-minute tour ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.
up vote 1 down vote favorite

To make my life a bit easier, I created a function DoQuery:

// DoQuery
function DoQuery($sql)
{
    global $cm_db_host,$cm_db_user,$cm_db_pass,$cm_db_name;
    $con = mysql_connect($cm_db_host,$cm_db_user,$cm_db_pass);
    if(!$con) return 1001;
    $db = mysql_select_db($cm_db_name);
    if(!$db) return 1002;
    $res = mysql_query($sql);
    if(!$res) return 1003;
    return $res;
    mysql_close();
}

The function works perfectly - it seems..

Here is how I use it:

 $res = DoQuery("UPDATE table SET column1='value 1',column2='value 2' WHERE id=1;");

 switch($res){
    case true:
      echo 'Response[success]ENDCMTAG';
      break;
    case 1001:
      die('Response[dberror1001]ENDCMTAG\r\n'.
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;
    case 1002:
      die('Response[dberror1002]ENDCMTAG\r\n'.
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;
    case 1003:
      die('Response[dberror1003]ENDCMTAG\r\n'.
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;  
 }

Now, when I purposely screw up the SQL statement, so case 1003 should be used, the case true is what is being executed.

According to the PHP manual (yes, I did look it up! ;) ) a MySQL INSERT, UPDATE .... will either return true or false. In my DoQuery function, whenever a MySQL function returns false, I output specific error codes instead, so I know where it went wrong. If no error occurs, $res should be true, and then the case true should be executed, right?

The problem is that it always outputs true, even if I change the SQL query to rubbish.

If I put the case true at the bottom of the switch, the case 1001 is always executed when everything goes OK - I know because I check the mysql_error() field, and its empty.

What am I doing wrong?

share|improve this question
    
Just as a side note, you shouldn't be creating a connection every time you want to do a query. –  Bojangles Aug 20 '11 at 13:21

2 Answers 2

up vote 1 down vote accepted

You are returning the value 1003, which evaluates to a "truthy" value when compared against a boolean. It will always evaluate to TRUE.

// Essentially it is this:
if (1003) {
   // I'm TRUE!!!
}

You cannot use a strict comparison === either, since in the true case, you are returning a MySQL result resource, not the boolean TRUE.

If you rearrange your switch() so that the valid resource case is at the bottom, your error cases will be correctly evaluated first.

 switch($res){
    case 1001:
      die('Response[dberror1001]ENDCMTAG\r\n'.
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;
    case 1002:
      die('Response[dberror1002]ENDCMTAG\r\n'.
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;
    case 1003:
      die('Response[dberror1003]ENDCMTAG\r\n'.
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;  

    // Valid result is the default case.
    default:
      echo 'Response[success]ENDCMTAG';
      break;
 }

However, this is a little weird and could cause problems if your function ever returns a different error value (if it is changed,) but the switch isn't updated. Instead, you should check for a true/false return on the value first and then pass it to the switch to evaluate error codes:

// Check if we have an Integer value. If we do, its an error code!
// If not, it's a result, or true!
if (is_int($res)) {
 switch($res){
    case 1001:
      die('Response[dberror1001]ENDCMTAG\r\n'.
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;
    case 1002:
      die('Response[dberror1002]ENDCMTAG\r\n'.
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;
    case 1003:
      die('Response[dberror1003]ENDCMTAG\r\n'.
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;  
 }
 // $res was valid. Success!
 else {
   // handle your valid results
 }
share|improve this answer
    
I tried using default at the bottom - didnt do it either. :S –  Jeff Aug 20 '11 at 13:21
    
@Jeff see my the later addition to check the true case outside the switch –  Michael Berkowski Aug 20 '11 at 13:23
    
Tried your last code - it is now always true even if I mess with the Statement. :S –  Jeff Aug 20 '11 at 13:28
    
The if check will always evaluate to true, right? –  Jeff Aug 20 '11 at 13:30
    
@Jeff Foolish me. Changed to if (!is_int($res)) since you're returning an integer value or the result resource. Sorry –  Michael Berkowski Aug 20 '11 at 13:30
up vote 1 down vote

Switch statements always do loose comparison.

So any number (but 0) will also evaluate to True.

Another improvement I would have on your code is make use of constants for the error codes.

define('QUERY_RESULT_SUCCESS', '1000');
define('QUERY_RESULT_CONNECTION_FAILED', '1001');
define('QUERY_RESULT_SELECTDB_FAILED', '1002');
define('QUERY_RESULT_QUERY_FAILED', '1003');

function DoQuery($sql)
{
    global $cm_db_host,$cm_db_user,$cm_db_pass,$cm_db_name;
    $con = mysql_connect($cm_db_host,$cm_db_user,$cm_db_pass);
    if(!$con) return QUERY_RESULT_CONNECTION_FAILED;
    $db = mysql_select_db($cm_db_name);
    if(!$db) return QUERY_RESULT_SELECTDB_FAILED;
    $res = mysql_query($sql);
    if(!$res) return QUERY_RESULT_QUERY_FAILED;
    return QUERY_RESULT_SUCCESS;
    mysql_close();
}

 $res = DoQuery("UPDATE table SET column1='value 1',column2='value 2' WHERE id=1;");

 switch($res){
    case QUERY_RESULT_SUCCESS:
      echo 'Response['.QUERY_RESULT_SUCCESS.']ENDCMTAG';
      break;
    case QUERY_RESULT_CONNECTION_FAILED:
      die('Response['.QUERY_RESULT_CONNECTION_FAILED.']ENDCMTAG'."\r\n".
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;
    case QUERY_RESULT_SELECTDB_FAILED:
      die('Response['.QUERY_RESULT_SELECTDB_FAILED.']ENDCMTAG'."\r\n".
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;
    case QUERY_RESULT_QUERY_FAILED:
      die('Response['.QUERY_RESULT_QUERY_FAILED.']ENDCMTAG'."\r\n".
      'MySQLError['.mysql_error().']ENDCMTAG\r\n');
      break;  
 }

Also note the double quotes around the \r\n since it has to be evaluated.

share|improve this answer
    
+1 for the loose comparison, I didn't know that! :) –  Jeff Aug 20 '11 at 13:22
    
If this is to be used for a query returning a result set, it doesn't return the resource in any way from which results could be fetched. Fine for INSERT/UPDATE though –  Michael Berkowski Aug 20 '11 at 13:25
    
It is to be used for INSERT and UPDATES :) –  Jeff Aug 20 '11 at 13:27
    
@Michael: see the OP's question :) –  PeeHaa Aug 20 '11 at 13:27

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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