Developing strong applications

I noticed that there are some functions such as is_int() or isset() or file_exists() or functions_exists() that are somehow very useful. When I write some code I do always think to every bad things that could happen to my site, but sometime I face problems such as:

Wait, this variable is set inside the PHP file; this means that no one could ever edit it, right? And if this "user" could edit it I would have a lot of more troubles because it would be able to manage the PHP file.

or

Does this really worth something to keep checking for file that should exists always?

Let's consider the following example which has no sense by its own, but will help me in order to make you understand what I'm talking about. PS: I exaggerated the code on purpose.

config.php

$doActions = true;

functions.php

function getID() {
    return $_COOKIE['userid'];
}

class eye {
    public static function see() {
        // gain the object the user is looking at
        return $object;
    }
}

index.php

class viewer {

    private $poniesSeen = 0;

    public function __construct() {
        /* Magic ponies are created here */
    }

    public function sawAPony($id) {
        if (file_exists('config.php')) {
            if (isset($doActions)) {
                if (is_bool($doActions)) {
                    if ($doActions) {
                        if (file_exists('functions.php')) {
                            if (function_exists('getID')) {
                                $id = getID();
                                if (!empty($id)) {
                                    if (!is_int($id)) {
                                        settype($id, 'int');
                                    }

                                    if (class_exists('eye')) {
                                        if (method_exists('eye', 'see')) {
                                            $o = eye::see();
                                            if (is_string($o)) {
                                                if ($o = 'pony') {
                                                    if (isset($this->poniesSeen) and is_int($this->poniesSeen)) {
                                                        ++$this->poniesSeen;
                                                        return true;
                                                    } else {
                                                        return false;
                                                    }
                                                } else {
                                                    return false;
                                                }
                                            } else {
                                                return false;
                                            }
                                        } else {
                                            return false;
                                        }
                                    } else {
                                        return false;
                                    }
                               开发者_StackOverflow } else {
                                    return false;
                                }
                            } else {
                                return false;
                            }
                        } else {
                            return false;
                        }
                    } else {
                        return false;
                    }
                } else {
                    return false;
                }
            } else {
                return false;
            }
        } else {
            return false;
        }
    }
}

Now, I think, which of this conditions should be kept and what thrown away because they have no sense at all? Why should I not check for them and why should I? Is there a golden-rule about this kind of obsessions?

---

This is why languages come with error messages (usually). It won't just crash silently if something goes wrong, it will tell you. So you can make reasonable assumptions about what you think should always be true, and if it's ever not true for some reason, you'll be informed.

The following checks are useless, IMO:

1. file_exists - IF the filename is known and you put it there. If the parameter is a variable, then this is OK.
2. is_bool, is_int, etc. If you insist on the type being correct, use === for comparison. The only one of these I ever use is is_array.
3. function_exists and the like - if you know the file is there, and you put the function in the file, you can safely assume the function exists. Again, there are special cases where this function is useful - yours is not one of them.

isset can be useful in some cases, like checking if an array contains a non-null value for a given key, or if a parameter was passed in $_REQUEST. There are other ways of checking that don't involve isset, but that's not my point.

If you can reasonably expect your assertions should true all the time, that it would take a data corruption or huge error on your part to make it false, don't bother checking. If something does go wrong once in a million tries, then you can catch the error and fix it for next time. The cost to your code readability is too great otherwise.

---

Generally I use these kind of checks sparingly, for the obvious reason of avoiding producing code like that above.

Examples.

Dont check for the existance of files you created yourself. Check for existence of a user's files that you might be trying to open for a write to or read from.

When it comes to type checking, again dont check the types of things that are not going to change. Do check types on user inputs from things like form elements. If a function needs an int then check it gets one....provided theres a chance it might not be getting one (user wrote his name or something)

---

This always depends on your application.

It doesn't make any sense to do this:

$foo = 3;
if (is_int($foo) && $foo > 3)
  // ...

However, anything that comes from outer sources should be well checked. Even if it comes from a database, where you think all the data is safe, think deep if something might go in the wrong direction or not.

So, checking is not always necessary. Taking a moment to think about whether the checking is necessary is advisable.

---

But there is one golden rule: Anything that can go wrong, will go wrong (eventually).

---

If you really need to have lots of checks like that, invert the checks and return immediately on failure, like this:

public function sawAPony($id) {
  if (!file_exists('config.php')) {
    return false;
  }

  if (!isset($doActions)) {
    return false;
  }

  // etc
}

That way you avoid the excessive indentation, and trying to match up braces. Ideally though, you probably need to find a way to refactor out all the checking somehow.

Joining them all up with && will not make the code any more readable, in my view.

As to which checks you need - you should check anything which your code depends on, provided those things aren't guaranteed by the system in which you're running.

Always check things over which you have no control (especially user input, external services etc). If you find yourself adding these checks in lots of places, refactor them out somewhere.

---

Charlie,

what you've done is very redundant and unnecessary. PHP handles most of that stuff for you (i.e. ensuring a number in a string is interpreted as an integer then later checking to see if it's an int just a few lines down when the variable hasn't even been used in between)

It's similar to going outside with not just clothes on, but with full space suit in a bullet proof car and driving 10 miles away from any object the car might crash into.

If your code wasn't just for example and actually had a purpose, or made sense, I could have rewrote it for you in away that would tell you the right type of "safe-guarding" should be, but I'll try anyway.

include("config.php"); 
include("functions.php"); 

class viewer {

    private $poniesSeen = 0;

    public function __construct() {
        /* Magic ponies are created here */
    }

    public function sawAPony($id) {
        $object = eye:see(); 
        if($object == 'pony') {
            $poniesSeen++; 
            return true; 
        }
        return false;    
    }
}

LOL. I think that's it. That's good enough. Worry about your program's logic, not program's program (if that makes any sense at all, probably doesn't).

Cheers,

Dan

---

Its a PHP design / programming style problem. Many web programmers like to use variables that may be or not may be previously declared and assigned initial value ("dynamic typing", I think is called).

And those variable can be of any type, or easily casted from one type to another ("string" to "int"), for example. PHP and other web programming languages allow this.

When I code my own files PHP, I usually code like it was static typing language like these:

---

mylibrary.php

// expects a 0 | 1 (boolean) query-string parameter, even if not present ?
// "pseudo-declare it"

/* bool var */ $myOption = false;
$temp = $_REQUEST['option'] ;
if ($temp != NULL) {
  $myoption = ((int)temp == 1);
}

/* int */ function myFunction () {
  // simulate variable declarations,
  // and initial values, like other languages
  /* int var */ $result = -1;

  global /* bool var */ $myOption;

  /* bool var */ $doSomething = false;

  while (...) {
    ...
    if ($myOption) {
      $doSomething = true;
    }
    ...
  }

  return $result;
}

---

These kind of verbose coding, allows me to avoid all those exists or type check functions.

---

I didn't go through all your code. But to answer your question, if you have complete control over your variables and no one else works on your system other than you and it doesn't accept any external user input you may consider skipping those checks.

But the whole problem of programming is we can never be sure of what our application is going to get put through.

A rule of thumb to go by would be, if you expect your application fails catastrophically if a variable is not vetted, then you should do checking. Don't you think it will be worth it?

It's true that the variables created inside the PHP file is not controllable by the user. But what about 2 or 3 levels down, or up? When it comes in contact with user input and then somewhere else it's passed back into your method? That's why it's better to validate and type check and assign default values to all your variables.

In the end, it's better to be safe than sorry.

---

:-D

First the things indicating a major problem for your app (e.g. old version of files needing removed classes). If something like this happen, just present a nice (or useful or both) 500 page to the user and alert yourself. Otherwise you may crash somewhere later which would be harder to debug.

• file_exists can be replaced by require "file"
• class|method|function_exists can be ommited if you can't do without them and you expect them to be there

isset is again not needed if you expect some variable that you have set in your code (just turn notices on in a dev environment).

Changing datatypes is useful when dealing with input (for example when you expect a number).

This all assumes you have good control over the code you're executing. If somebody untrusted can modify a part of your code, then good luck.

---

First of all we cant get rid of checking for conditions in any small projects. If u need lots of checking, first write all them neatly. Then consider refactoring the code based on the conditions that u can check together, for example

if(a!=0){
   if(b!=0){
      // do this
   }

} can be re written to

if(a!=0 && b!=0) {
   //do this
}

(for the sake of easiness I made it too simple).