Should my if statement work, is it structured properly?

Question

if (file_exists($cachefile) && (time() - $cachetime < filemtime($cachefile)) && $_SERVER['REQUEST_URI'] != "login" || "thankyou" || "confirm") 

Should this code work, operators wise and syntax wise?

Answer 1

This part won't work as you (probably) want:

$_SERVER['REQUEST_URI'] != "login" || "thankyou" || "confirm"

Firstly, REQUEST_URI will never be equal to any of these values - it will have at least an additional /. If your script is named test.php, REQUEST_URI will contain at least /test.php (see below what else it might contain). Take a look at the REQUEST_URI variable to see how it is structured.

Also, REQUEST_URI is treacherous here: it will contain any additional query strings so it will be possible to confuse the comparison by adding an arbitrary ?name=value to the URL.

Do a phpinfo() to find something more fitting than REQUEST_URI, e.g. SCRIPT_NAME.

Secondly, the "or" operations you are doing will be applied to the values before the comparison is done.

You probably want something like this:

!in_array($_SERVER['SCRIPT_NAME'], 
          array("/login.php", "/thankyou.php", "/confirm.php"))

Answer 2

Even if it does, I'd create a method for this kind of "complex" conditions.

The methods body would look something like this:

if(!condition1)
  return false;

if(!condition2)
  return false;

if(!conditionN)
  return false;

return true;

Edit: The reason is just readbility, you can name those condition methods appropriately and the methods themself are way more readable than a very long if statement.