The most common programming mistakes 1

I'll show you a few examples where code is unnecessary complicated and which could be written better, easier and in some cases even there can be (even minimally) improved performance.

I'll show you bad examples and, if necessary, I'll put here a solution that you wouldn't be ashamed of. Some of you may even remember your own beginnings in programming, and for some it may help get rid of unnecessary code. Thanks for reading and enjoy the articles!

Let's start with very simple examples that don't appear anywhere in practice. Well, you'd be surprised...

Inappropriate variable names

A) Using meaningless names

Usually, it's easier to write a random variable name so that we don't spend too much time coming up with a name. These are, for example, the following cases:

1: $aaa = 10;
2: $x1 = array("a", "b");
3: $x2 = array("c", "d");
4: $x163 = "some string";

Why come up with some names when I use it somewhere anyway and don't deal with it anymore? The problem occurs when you make a larger script, so you'll then ask yourself:

6336: if ($x163 !== fooBar()) {
6337:    // what is that $x163 ?!
6338:    ...
6339: }

In addition, poor colleagues, if they see something like this in a joint project.

B) Mixing languages

This is the case when you use, for example, English and Spanish in a name.

$numberOfPersonas
$numeroDeCars

It isn't only a specific variable, but one language should be used throughout a whole project, including function names, etc. Otherwise, such pieces of code will be created:

if (count($inputData) > $ourSuperConfig["dataStorage"]["numeroMaximoDeRegistros"]) {
  ...
}

It's usually good to write a whole program in English. On the one hand, there are also all keywords in English, so if you'd like to publish an open-source program, other developers will understand it better.

C) Choosing a bad name

Another bad situation can occur if you use a bad name. I'll exaggerate it as an example.

$charlesAge = array(-4, 10, "abc");

It's nice that you chose a certain name and even in one language, but under the name charlesAge you won't probably expect an array with three different values (and even one negative and the third string).

Exceptions are, for example, $i and $j iteration variables, which are used when traversing an array.

Using too many elseif conditions

This is sometimes a case when, for example, creating dynamic websites, where a title is passed through a URL parameter.

$page = $_GET["page"];

if ($page === "home") {
  include "home.php;"
} elseif ($page === "about") {
  include "about.php";
} elseif ($page === "contact") {
  include "contact.php";
} elseif ($page === "sitemap") {
  include "sitemap.php";
} /* etc ... */ else {
  include "404.php";
}

Not only it's quite confusing (imagine 100 different pages, for example), but you'll waste a bunch of lines and even if you wanted to change a $page variable's name, you'll have to rewrite all conditions.

This can be solved very easily by creating a whitelist, or list of allowed values:

$page = $_GET["page"];
$allowedPages = array("home", "about", "contact", "sitemap");

if (isset($allowedPages[$page])) {
  $file = "{$page}.php";
  include $file;
} else {
  include "404.php";
}

In addition, the array can be easily expanded. For example, if you want to have a different file name, you can create an associative field:

$page = $_GET["page"];
$pages = array(
  "home" => "home.php",
  "about" => "aboutMe.php",
  ...
);

if (isset($pages[$page])) {
  include $pages[$page];
} else {
  include "404.php";
}

We can extend the array with additional elements that will be used in each page.

<?php

$page = $_GET["page"];  // we'll assume that the page parameter in the URL always exists

$pages = array(
  "home" => array(
    "file" => "home.php",
    "title" => "Home",
    "needLogin" => FALSE
  ),

  "admin" => array(
    "file" => "admin/index.php",
    "title" => "Administration",
    "needLogin" => TRUE
  ),

  "404" => array(
    "file" => "404.php",
    "title" => "Error 404, page not found"
  ),

  "onlyForLoggedUsers" => array(
    "file" => "onlyForLoggedUsers.php",
    "title" => "Only for logged users"
  )
);

if (isset($pages[$page])) {
  $pageData = $pages[$page];

  if (isset($pageData["needLogin"]) && $pageData["needLogin"]) {

    // if the page requires a login, a check will be performed automatically
    if (!isset($_SESSION["userId"])) {
      $pageData = $pages["onlyForLoggedUsers"];
    }
  }
} else {
  $pageData = $pages["404"];
}

?>
<html>
  <head>
    <meta charset="UTF-8" />
    <title><?php $pageData["title"]; ?></title>
  </head>
  <body>

  <?php

  include $pageData["file"];

  ?>

  </body>
</html>

code

This example can work for small and very simple sites. If you want to add different form processing, it's better to compile the code a little differently (because form processing should take place before any HTML listing).

Replacing array values

If for some reason you need to change values in an array, you can do it as follows:

$numbers = array(1, 2, 3, 4, 5);
$count = count($numbers);
for ($i = 0; $i < $count; $i++) {
  $numbers[$i] = $numbers[$i] * 2;
}

code

The line with multiplication can be simplified using a reference:

foreach ($numbers as &$number) {
  $number = $number * 2;
}

code

Notice a & character (ampersand) before the $number variable. It doesn't copy a content of an element, but saves a reference to a memory location. This means that if we adjust the $number variable's value, a value of a given element in the array will also change.

Too much branching

Too much of nested conditions can be seen, for example, in the form processing. A simple example that could represent user registration:

if ($_POST) {
  if (!empty($_POST["name"])) {
    if (empty($_POST["pass"])) {
      if (empty($_POST["repass"])) {
        if (!empty($_POST["email"])) {
          if ($_POST["pass"] === $_POST["repass"]) {
            if (strlen($_POST["name"]) > 5) {
              // registration completed
            } else {
              echo "Your name is too short;
            }
          } else {
            echo "Passwords don't match";
          }
        } else {
          echo "Please enter your email"
        }
      } else {
        echo "Please confirm your password";
      }
    } else {
      echo "Please enter your password";
    }
  } else {
    echo "Please enter your name";
  }
}

// and I didn't add an email check, invalid characters check and bunch of other potential conditions

code

Getting rid of this complicated code is very easy. This method even allows us to save more errors, so there is no need to bother the user with one message every time until he decides to close the page.

$errors = array();
if ($_POST) {
  if (empty($_POST["name"])) {
    $errors[] = "Please enter your name";
  }
  if (empty($_POST["pass"])) {
    $errors[] = "Please enter your password";
  }
  if (empty($_POST["repass"])) {
    $errors[] = "Please confirm your password";
  }
  if (empty($_POST["email"])) {
    $errors[] = "Please enter your email";
  }

  if (empty($errors)) {  // if there are no errors yet, we can continue
    if (strlen($_POST["name"]) <= 5) {
      $errors[] = "Your name is too long";
    }
    if ($_POST["pass"] !== $_POST["repass"]) {
      $errors[] = "Passwords don't match";
    }

    if (empty($errors)) {
      // registration completed
    }
  }
}

code

You can then simply list individual errors using a loop.

if (!empty($errors)) {
  echo "<ul>";
  foreach ($errors as $error) {
    echo "<li>{$error}</li>";
  }
  echo "</ul>";
}

code2

Of course, the solution also includes some nesting, but the code suddenly looks much clearer. It's basically a series of certain conditions, where a state is determined at the end (there are errors/there are no errors). This is because if a name isn't filled in, both the "please enter your name" and "your name is too short" messages aren't displayed at once.

Counting elements in an array during a loop

It's often seen in code that the count or sizeof function is used (both do the same thing) when traversing an array.

for ($i = 0; $i < count($fields); $i++) {
  // some code
}

code

When traversing a loop, the condition is tested each time. This means that the count() function is called every time to count elements in an array. It's not a mistake, it just counts again unnecessarily every time. Maybe it could show at performance if the array contained a million values, for example. The best way is to count elements before the loop and assign only a variable into the condition.

$fieldsCount = count($fields);
for ($i = 0; $i < $fieldsCount; $i++) {
  // some code
}

code

That's all from the first article. Thank you for reading and if you're interested, I'll be happy to try to write more (for example, mistakes when working with data from a database, etc.).