Scripting Bugs

[admin]

Whoops, confirmed! Yep, coming...

[nerdweed]

It appears that the second condition is unnecessarily evaluated even if not required

if (1==2 && property("#11", "<curitem>") == "Picture") {
echo "Should never execute - but I checked both" ;
} else {
echo "I took a little longer to come here" ;
}

[PeterH]

I would not see this as a (kind of) bug.

I think XY does what's written: it calculates the value of the boolean expression - and then decides the 'if'.
Though it might be a bit more performant, if the 2nd operand of an 'and' will not be calculated if the first is '0'. But bad (for you), if the operands are calculated right to left (Don't say this is not allowed... )

[nerdweed]

If it's right to left, I can simply switch them. But it doesn't appear to be that way either

[admin]

This optimization could be too smart. Look at this (air code). You might want to try the second write even if the first failed.

Code: Select all

if (writefile($file1, $data1) == 0 && writefile($file2, $data2) == 0) {
  echo "Both failed!";
}

[nerdweed]

That air code would say both failed even if one succeeded. If I would want one stuff to be completed regardless of other, I wouldn't put it in relation to the first.

It would be good if that smartness can be put in place - this would avoid expensive operations to the right if the success/failure of the condition has already been decided.

Edit: We can split the operation in two (but that requires copying the else as well)

[admin]

No. Try this:

Code: Select all

$file1 = "a*.txt"; $file2 = "b.txt"; $data1 = "X";
  if (writefile($file1, $data1) == 0 && writefile($file2, $data1) == 0) {
   echo "Both failed!";
  }

[admin]

This is what I meant by "This optimization could be too smart.". I cannot know what the code intended or expects. And I think the default assumption is that both parts of the AND-term are processed.

You can easily optimize your code by nesting:
Instead of

Code: Select all

if A and B then

you do:

Code: Select all

if A then
 if B then

[nerdweed]

Correct. After numerous edits.
1 and 1 = 1
0 and 0 = 1

I should use the Or operator.

Edit: Note to self - Read the truth tables again.

[admin]

0 and 0 = 1 ???

In which part of the universe do you live?

[nerdweed]

Oh, I was correct earlier. I thought I boo-ed earlier but boo-ed on the second instance.

In which part of the universe do you live?

Gravity pushed me at that instance.

But if first is false, why should the second be checked - hasn't the failure been determined.

If (1==0 && 1==1) {
;
}

When first is true, why check for the second.
if (1==1 || 1==0) {
;
}

Shouldn't this be short circuit operators like in PHP

The problem with nesting is the Else part.

[PeterH]

But if first is false, why should the second be checked - hasn't the failure been determined.

Basically this is a bool expression - and is simply calculated by bool logic. I.e. it is and AND instruction - and is executed even if the first operand of it is false.

For your wish the evaluation logic would have to be expanded to test the 1st operand for false, and in this case abend the bool calculation. This would be analog to the "nested if" Don gave as an example. And would have the implications Don mentioned.

[LittleBiG]

I have a real life example where I could have used what nerdweed suggests. In a script I get the recent paths and I would like to create a list about the existing folders. If the actually checked folder is a network folder, I want to add it to my list, without further check, assuming that it is existing. (because the exists(network_folder) slows the script down significantly) I tried this:

Code: Select all

if (substr($NextPath,0,2) == "\\" or exists($NextPath) > 0) {add the path to my list}

If the first part is true, no need the run the second part, the whole expression will be true, because of the "or". But XY runs the second part anyway, slowing down the script significantly. I had to rewrite the script in an unfortunate way:

Code: Select all

if (substr($NextPath,0,2) == "\\") {add the path to my list}
elseif (exists($NextPath) > 0) {add the path to my list}
else {not accessible path, go to the next one}

As you can see I had to double the adding part. The "add the path to my list" part is not as simple as I show it in my example, so I have to keep the two part in sync manually.

[admin]

No, what you did is good coding. To avoid the doubling of code simply use a little local flag:

Code: Select all

if (substr($NextPath,0,2) == "\\") {$add = 1;}
elseif (exists($NextPath) > 0) {$add = 1;}
else {$add = 0;}

if ($add == 1) {add the path to my list}

[LittleBiG]

To avoid the doubling of code simply use a little local flag

Why is it so obvious after you have written it down? And why not before? Thanks!