Jump to content

Important "if" statement relying on mySQL


StevenOliver
 Share

Recommended Posts

I have a "best practices" question.

Visitors are limited to a maximum of 5 shipping labels per day, so mySQL counts how many were created in the past 24 hours and logs them.

Which "style" is better?

###  Style #1  #####################

$labelCounter = $db->prepare("SELECT count(*).. // mySQL counts labels

if($numberoflabels < 5) {

// RUN BIG CURL SCRIPT IF LESS THAN 5 LABELS
// LONG CURL SCRIPT GOES HERE

$log = $db->prepare("INSERT into label_counter... // log the good label

} else { // don't run big curl script

echo "Sorry visitor, no label could be created";

}

###  Style #2  #####################

$too-many-labels = FALSE;

$labelCounter = $db->prepare("SELECT count(*) // mySQL counts labels

if($numberoflabels > 4) {
$too-many-labels = TRUE;
}

if ($too-many-labels == FALSE) {

// RUN BIG CURL SCRIPT IF LESS THAN 5 LABELS
// LONG CURL SCRIPT GOES HERE

} else { // don't run big curl script

echo "Sorry visitor, no label could be created";

}

Style # 1 seems more direct, but Style 2 somehow "looks safer." Which is really better? What are your thoughts, please?

Link to comment
Share on other sites

Why set a superflous boolean variable when you have a perfectly good boolean expression already. All you are doing is adding an extra step

$too_many_labels will have the same value as  $numberoflabels > 4

I'd go for option 1 or option 3, which would be

// style 3
$labelCounter = $db->prepare("SELECT count(*) ... "); // mySQL counts labels

if($numberoflabels > 4) {
     echo "Sorry visitor, no label could be created";
}
else {
    // RUN BIG CURL SCRIPT IF LESS THAN 5 LABELS
    // LONG CURL SCRIPT GOES HERE

    $log = $db->prepare("INSERT into label_counter... // log the good label 
}  

 

  • Like 1
Link to comment
Share on other sites

Know that with either of those it's possible to make more than five with concurrent users.  If 4 labels have been made and two more requests come in and process very close together they might both pass the check and add a label resulting in 6 total labels at the end.  If you're concerned about that, then the solution is to use a lock to prevent the concurrent requests.

 

Link to comment
Share on other sites

4 hours ago, Barand said:

I'd go for option 1 or option 3...

Thank you! You just now reminded me of a question I've had since day one:

On an if/else statement, if one condition is more likely to happen than the other, should that condition go first? Or, does it absolutely not matter.

Using my example, it's extremely unlikely a visitor will try to print more than 5 labels (less than 1/100 chance). Would I want the first condition of the if/else statement to be the least likely scenario:

if ($numberoflabels > 4) {
echo 'sorry';
} else {
// proceed everything fine
}  

-or-

Would I want the first condition to be the most likely scenario:

if ($numberoflabels < 5) {
// proceed everything fine
} else {
echo 'sorry';
}  

Does one version over the other operate faster?

 

Link to comment
Share on other sites

3 hours ago, StevenOliver said:

On an if/else statement, if one condition is more likely to happen than the other, should that condition go first? Or, does it absolutely not matter.

It doesn't matter.  Any potential performance difference would be unmeasurable.

Put them in which ever order makes the code easiest to read and understand.

 

Link to comment
Share on other sites

21 hours ago, kicken said:

It doesn't matter.  Any potential performance difference would be unmeasurable.

Put them in which ever order makes the code easiest to read and understand.

However, I would strongly suggest putting the error condition first as Barand did in his examples. The reason is that it makes it easier to "see" the condition that creates the error. Plus, if your error condition does a redirect, you don't even need to enclose the main code in an else clause. This makes the code easier to read/maintain.

if($foo == 'bar) {
    include('some_error_page.php');
    exit;
}
//Continue with the main logic w/o else clause

Also as Barand's stated, there is no reason to create a variable when you have a perfectly good condition. But, if you do need to create a flag based on a condition, do not do it this way

if($numberoflabels > 4) {
$too-many-labels = TRUE;
}

Instead do this:

$too-many-labels = ($numberoflabels > 4);
Edited by Psycho
Link to comment
Share on other sites

This thread is more than a year old.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

 Share

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.