Strong application development

Question

I noticed that there are some functions, such as is_int() or isset() or file_exists() or functions_exists() , which are somehow very useful. When I write code, I always think of any bad things that can happen to my site, but sometimes I encounter problems such as:

Wait, this variable is being set inside the PHP file; that means no one can edit it, right? And if this “user” could edit it, I would have more trouble because he could manage the PHP file.

or

Is it really worth it to constantly check the file, which should always exist?

Let's look at the following example, which does not make sense on its own, but helps me to make you understand what I'm talking about. PS: I specifically exaggerated the code.

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 these conditions should be preserved and what should be thrown away, because they have no meaning? Why shouldn't I check them and why? Is there a golden rule about these obsessions?

Answer 1

This is why languages ​​come with error messages (usually). He will not just crash, if something goes wrong, it will tell you. In this way, you can make reasonable assumptions about what you think should always be true, and if it is ever to be wrong for any reason, you will be informed.

The following checks are useless IMO:

• file_exists - If the file name is known, and you put it. If the parameter is a variable, then this is normal.
• is_bool , is_int , etc. If you insist on the correct type, use === for comparison. The only thing I used is is_array .
• function_exists and the like - if you know that there is a file, and you put the function in the file, you can safely assume that the function exists. Again, there are special cases when this feature is useful - yours is not one of them.

isset can be useful in some cases, for example, checking if the array contains a non-empty value for the given key or if the parameter was passed in $ _REQUEST. There are other verification methods that do not include isset , but that is not my point.

If you can reasonably expect your claims to be true all the time, that you will need data corruption or a huge mistake to make it false, don't bother checking. If something goes wrong, just in a million attempts, you can catch the error and fix it next time. Otherwise, the cost of reading your code is too high.

Answer 2

I usually use these checks sparingly, for the obvious reason of avoiding code like the one above.

Examples.

Do not check for files that you created yourself. Check for user files that you might be trying to open for writing or reading.

When it comes to type checking, again don't check the types of things that won't change. Check types on user inputs from elements such as form elements. If the function needs an int, then check if it receives ... provided that it is a chance that it may not receive it (the user wrote his name or something else)

Answer 3

It always depends on your application.

This makes no sense:

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

However, everything that comes from external sources should be well verified. Even if it comes from a database where you think that all the data is safe, think if something could go in the wrong direction or not.

Therefore, verification is not always necessary. It is advisable to think about the need for verification.

---

But there is one golden rule: Everything that can go wrong will go wrong (in the end) .

Answer 4

If you really need to have many such checks, invert the checks and immediately return to the error, for example:

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

This way you avoid over-indentation and try to align the brackets. Ideally, however, you probably need to find a way to refactor all the checks in some way.

Combining everyone with && will not make the code more readable, in my opinion.

As for the checks that you need, you should check everything your code depends on if these things are not guaranteed by the system in which you work.

Always check that you have no control (especially user input, external services, etc.). If you find yourself adding these checks in many places, reorganize them somewhere.

Answer 5

Charley,

what you have done is very redundant and unnecessary. PHP handles most of this stuff for you (i.e., ensuring the number in the string is interpreted as an integer, then checking to see if it is int just a few lines down when the variable is not even used between them)

It looks like going outside with not only clothes, but also with a full space suit in a bulletproof car and driving 10 miles from any subject that the car could crash into.

If your code was not just for example and really had a purpose or made sense, I could rewrite it for you far away, to tell you that the correct type of "safe protection" should be, Anyway, try it.

  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 this is right. This is good enough. Worry about your program logic, not your program (if that makes sense at all, probably not).

Greetings

Dan

Answer 6

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

And these variables can be of any type or it is easy to pour from one type to another (for example, "string" to "int"). PHP and other web programming languages ​​allow this.

When I code my own PHP files, I usually code as a static input language, for example:

---

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

---

Such verbose coding allows me to avoid all existing ones or test validation functions.

Answer 7

I did not go through all your code. But to answer your question, if you have full control over your variables and no one else works on your system except you, and it does not accept any external user input, you can skip these checks.

But the whole programming problem is that we can never be sure of what our application will do.

A rule of thumb that could be followed would be if you expect your application to be catastrophically catastrophic, if the variable is not checked, then you should check. Don't you think it's worth it?

It is true that variables created inside a PHP file are not user controlled. But what about 2 or 3 levels down or up? When it comes in contact with user input and then somewhere else, does it go back to your method? Therefore, it is better to check and print the check and assign default values ​​to all your variables.

After all, it's better to be safe than sorry.

Answer 8

: - D

First of all, this indicates a serious problem for your application (for example, an old version of files requiring the removal of classes). If something like this happens, just provide the user with a nice (or useful or both) 500 page and report it. Otherwise, you might work somewhere later, which would be harder to debug.

• file_exists can be replaced with require "file"
• class|method|function_exists can be circumvented if you cannot do without them, and you expect them to be there.

isset is not required again if you expect some variable that you set in your code (just enable notifications in the dev environment).

Changing data types is useful when working with input (for example, when you expect a number).

All of this assumes that you have good control over the code that you are executing. If someone does not trust, can change part of your code, then good luck.

Answer 9

First of all, we cannot get rid of checking conditions in any small projects. If you need a lot of checking, write all of them carefully first. Then consider code refactoring based on conditions that can be checked together, for example

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

} can be written to

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

(for the sake of simplicity, I did it too simply).