global $issues

Recently I realized that I don’t hate PHP, as I thought for many years, I just hate people who code in PHP. And before lynch mob starts knocking on my door, let me elaborate.

PHP requires minimal learning curve to get in and whip together a small website (props where it’s due). Unfortunately, the ease-of-use has one major flaw that is a root cause of my frustration. Way too many people, who shouldn’t be allowed to use the computer for any other purpose than to play solitaire, start coding in PHP. Without any previous exposure to programming. Without any knowledge of basic theory. Without a clue. And they like it. And what’s worst of all, they often succeed in their first endeavor. And they crave for more.

Now, if everyone in the world wants to build a personal website by themselves — all the power to them. It’s when the will without a clue attitude (I should patent that) moves into the corporate world, that’s when the problems start.

At $work, large part of our days is spent on improving someone else’s code. People come to us to make code more flexible, scalable, or just plainly to make it work. Recently we inherited the code for a relatively small project, with a very large following. If you haven’t guessed by my long preamble yet, the project was coded in PHP.

Now, I am no PHP expert by any stretch of imagination, I would probably rank myself more on the novice part of the scale when it comes to PHP, but since I had a few spare minutes, I decided to take a look at the code. My first (and all the following) impressions of it can only be expressed in non-alphanumeric characters.

One of the main issues I have with PHP as a language is a lack of lexical scope. As my co-worker appropriately noted “PHP only has 2 scopes: global and wrong”. Well, the developer of the code solved the problem by removing “wrong” scope. Every variable in the code was explicitly defined as “global”. Every. Single. Variable. I honestly asked around if “global” in PHP means the same thing as does in every other programming language, because I couldn’t imagine anyone going thought the effort of eliminating any notion of scoping. In all fairness, later I realized that there was a reason for that, the developed didn’t know how to pass variables to and from functions.

After I forced myself out of scope-induced shock, I started looking at the actual logic of the application.

require_once 'functions.php';
  $vars['foo'] = $_POST['foo'];
  ... # read POST vars into $vars
  init();
  if ($_SESSION['id']) {
      load();
  }
  $query = "UPDATE table_foo .... "; # update with $var data
  $result = mysql_query( $query, $db );

At the first glance the logic is pretty simple. Check the session for ID, if exists gather some data and update the appropriate table with posted data. The fact that the developer didn’t use PDO (yes, even I know what it is) for database connection was mildly concerning, but since all the variables were escaped, I let it fly. What caught my attention was load() function before an update. There can be plenty reasons for it, but collection data right before the update while already having ID just didn’t seem right. So, to double-check myself, I opened functions.php.

function init() {
    global $id;
    global $var;
    if ($_SESSION['id']) {
        $id = $_SESSION['id'];
    }
    $query = "INSERT into table_foo .... "; # insert partial $var data
    $result = mysql_query( $query, $db );
    if ( $result ) {
        load();
    }
}
function load() {
    global $id;
    global $var;
    if ($_SESSION['id']) {
        $id = $_SESSION['id'];
    }
$query =  "SELECT * FROM table_foo where ID=$id";
    $result = mysql_query( $query, $db );
if ( $row = mysql_fetch_assoc( $result ) ) {
        $var = $row;
    }  else  {
        init();
    }
}

It took me good 30 min to triple-check that the logic in front of me was not a figment of my sick and twisted imagination. Because according to my eyes, the flow of each request was as follows:

1. Read $_POST variables
2. Call init()
3. Check if ID exists in $_SESSION (in init())
4. Attempt to insert $vars into table (in init())
5. If insert succeeded, call load()
6. Check if ID exists in $_SESSION and select (!!!) the same row you just inserted regardless of whether ID exists in $_SESSION (in load())
7. If select doesn’t return anything (for example, let say, I don’t know, $_SESSION[‘id’] doesn’t exist!) call init() (step 2)
8. After you exit init(), check if ID exists in $_SESSION
9. Call load() (step 5)
10. Update the same row you just inserted in step 4 with data you set in step 1

At minimum, every request ran 4 queries, 2 of which were identical selects of the data you already have, and other 2 are insert and update of, get this, the same data. Not only that, but if you enter the page without ID in $_SESSION, you still run the selects to make sure the data for not-existent ID is not in the database. How anyone found this logical is beyond me.

Now, I can’t really blame PHP for incompetence of coders, no matter how much I want to. There are plenty of poor programmers in every language. But the trend is that “anyone can learn PHP overnight”, and it’s those people who are responsible for producing abysmal code that is acceptable by people only concerned with their sites “working”. The fact that “working” doesn’t scale or is not flexible for growing needs is not a topic of conversation until it becomes an issue. And that’s usually when we come in, since the same people who wrote the initial code don’t have the knowledge or the skills to resolve those problems. Talk about job security.