PHP script misbehaving

Question

I have a small script that defines a variable called $prPrice. This script is not working in the way that I intend, as in some of the products that contain a certain department code are display prices relating to other department codes. So i assume that the way i have initially put this together is somehow wrong.

My question would be this, can anyone spot a problem with, or provide suggestions for bettering the following simple php script:

// Cost plus 2.5%
if ($prDept = (204 || 205 || 209 || 1501 || 1601 )) {
    if ($_SESSION['tax'] == "on" || !isset($_SESSION['tax']))
        $prPrice = ((($prCost1 / 1.14) * 1.025) * 1.14);
    else if ($_SESSION['tax'] == "off" && $prTaxable == "1")
        $prPrice = ($prCost1 * 1.025);
    else if ($_SESSION['tax'] == "off" && $prTaxable == "0")
        $prPrice = (($prCost1 / 1.14) * 1.025);

}

you are not optimizing anything, your script doesn't work right. en.wikipedia.org/wiki/Program_optimization — dvir
– dvir, Commented Oct 16, 2011 at 19:04
"Optimizing" is not a synonym for "fixing". And how are we supposed to deduce what you intended from code that, by your own admission, doesn't do that thing? — Lightness Races in Orbit
– Lightness Races in Orbit, Commented Oct 16, 2011 at 19:10

Amber · Accepted Answer · 2011-10-16 19:07:59Z

5

$prDept = (204 || 205 || 209 || 1501 || 1601)

That is not how this works in PHP. It really needs to look like...

$prDept == 204 || $prDept == 205 || $prDept == 209 || $prDept == 1501 || $prDept == 1601

or alternatively,

in_array($prDept, array(204, 205, 209, 1501, 1601))

answered Oct 16, 2011 at 19:07

Amber

531k89 gold badges643 silver badges558 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Lightness Races in Orbit Over a year ago

This mistake comes from the common English error "my A can be X or Y or Z", which in fact should read "my A can be X, or my A can be Y, or my A can be Z". Direct correlation. Try not to think of programming languages as English, because they are not as forgiving when you remove logical consistency from your expressions.

Amber Over a year ago

(If you really want to think of an English version, the proper one for relating to programming languages would be "my A can be one of the values X, Y, or Z", where X,Y,Z is a set (or an array, in this case).)

Craig van Tonder Over a year ago

@Amber I see where you are going with this, i think what i was trying to accomplish would be something more like the latter. I appreciate your input, and thank again!

Lightness Races in Orbit Over a year ago

@Amber: I was trying to make a more general grammatical correlation, but, sure.

Craig van Tonder Over a year ago

@Tomalak Say for instance, there were two sets of this script, each relating to different departments. Would it be better to arrange this as an if, else if. Or have it as two seperate if's?

Mob · Accepted Answer · 2011-10-16 19:10:08Z

2

if ($prDept == 204 || $prDept == 205 || $prDept == 209 || $prDept == 1501 || $prDept == 1601) {
    if ($_SESSION['tax'] == "on" || !isset($_SESSION['tax'])) {
          $prPrice = ((($prCost1 / 1.14) * 1.025) * 1.14);
    } else if ($_SESSION['tax'] == "off" && $prTaxable == "1") {
          $prPrice = ($prCost1 * 1.025);
    } else if ($_SESSION['tax'] == "off" && $prTaxable == "0") {
          $prPrice = (($prCost1 / 1.14) * 1.025);
    }   
}

answered Oct 16, 2011 at 19:10

Mob

11.1k7 gold badges43 silver badges58 bronze badges

Comments

Sorin · Accepted Answer · 2011-10-16 19:39:30Z

1

I would write this as:

$departments = array(204 , 205 , 209 ,1501, 1601);

if (in_array($prDept, $departments)) {
 ...
}

answered Oct 16, 2011 at 19:39

Sorin

5,4662 gold badges21 silver badges45 bronze badges

Collectives™ on Stack Overflow

PHP script misbehaving

3 Answers 3

5 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

5 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related