Developing strong applications

Question

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;
                                    }
                                } 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?

Answer 1

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.

Answer 2

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)