ManChuck
Nested If’s the root of all evil
Ok so the title is an over exaggeration but I need a simple way to express how much I HATE when I see code with dozens of closing brackets because the developer was trying to fix the Unexpected $end error. Simple control structures not only help to keep code readable, it will also help in performance because you can escape code blocks with ease. To illustrate, below is an example of a function I have seen in my adventures in programming:
- function isAuthorized($resource)
- {
- $allowed = false;
- if ($_SESSION['loggedin'] == 1) {
- $user = $_SESSION['username'];
- if (!empty($user)) {
- $result = mysql_query('SELECT * FROM user_roles WHERE user = "' . $user . "'");
- if ($result) {
- while ($resArray = mysql_fetch_array($result)) {
- if ($resource == $resArray['role']) {
- $allowed = true;
- }
- }
- } else {
- throw new Exception('No valid results');
- }
- } else {
- throw new Exception('User name is invalid');
- }
- }
- return $allowed;
- }
Lets just ignore the security hole and performance issues with the MySql query, just look at the structure starting with the while loop. What if that user had 100 roles assigned and the role we wanted is the 1st record? We set $allowed to true on the 1st iteration then spend 99 iterations doing nothing. We can either break the loop or just add the $allowed to the condition. I suggest adding the $allowed to the loop which will clearly state that we are looking for $allowed.
- while (($resArray = mysql_fetch_array($result)) && !$allowed) {
- if ($resource == $resArray['role']) {
- $allowed = true;
- }
- }
Ok a little better we now have a chance of breaking the loop if we find the resource early. We can still optimize this function but returning true or false based off our conditionals:
- function isAuthorized($resource)
- {
- if (!$_SESSION['loggedin'] == 1) {
- return false;
- }
- $user = $_SESSION['username'];
- if (empty($user)) {
- throw new Exception('User name is invalid');
- }
- $result = mysql_query('SELECT * FROM user_roles WHERE user = "' . $user . "'");
- if (!$result) {
- throw new Exception('No valid results');
- }
- while ($resArray = mysql_fetch_array($result)) {
- if ($resource == $resArray['role']) {
- return true;
- }
- }
- return false;
- }
We gain a many advantages here. First PHP will not have to allocate memory for the $allowed and $user variables since they will not get called if the user is clearly not logged in. We also gain greater op-code usage by not having the parse read in else statements. The big advantage to this is how easy it is to read and make a change. Lets say we wanted to add a check for if the users session expired, by following the 1st example we would have to add a whole new block as follows:
- function isAuthorized($resource)
- {
- $allowed = false;
- if ($_SESSION['loggedin'] == 1) {
- if (date_add($_SESSION['loggedin'], '+30 min') < new DateTime("now")) {
- $user = $_SESSION['username'];
- if (!empty($user)) {
- $result = mysql_query('SELECT * FROM user_roles WHERE user = "' . $user . "'");
- if ($result) {
- while ($resArray = mysql_fetch_array($result)) {
- if ($resource == $resArray['role']) {
- $allowed = true;
- }
- }
- } else {
- throw new Exception('No valid results');
- }
- } else {
- throw new Exception('User name is invalid');
- }
- } else {
- throw new Exception('Session has expired');
- }
- }
- return $allowed;
- }
With our cleaned up structure we just add the following lines like so:
- function isAuthorized($resource)
- {
- if (!$_SESSION['loggedin'] == 1) {
- return false;
- }
- $user = $_SESSION['username'];
- if (empty($user)) {
- throw new Exception('User name is invalid');
- }
- if (date_add($_SESSION['loggedin'], '+30 min') < new DateTime("now")) {
- throw new Exception('Session has expired');
- }
- $result = mysql_query('SELECT * FROM user_roles WHERE user = "' . $user . "'");
- if (!$result) {
- throw new Exception('No valid results');
- }
- while ($resArray = mysql_fetch_array($result)) {
- if ($resource == $resArray['role']) {
- return true;
- }
- }
- return false;
- }
As we can see keeping the code structured helps us understand what a function is supposed to be doing, improves our opcode performance, and helps keep making changes to a function that much easier
New Look
Well as you can see, MANCHUCK has a new look. I switched over to Word Press because really, it takes a long time to build your own blog from scratch (which is what I was hoping to do with the old site). It took about a day to install and configure the new look. Hope you all enjoy
Shout out to Themetation for creating this theme. Thanks for helping save me some time with messing around with CSS