Top 5 worst coding practices

Standard

In normal literature, it is easy to read, difficult to write. But in programming, it is the reverse. It is easy to write, difficult to read. So no wonder in the programming world, we keep producing monkeys. In literature, poorly written books get trashed openly. Poorly written code becomes technical debt to the software, bury deep into the system, forgotten by people due to turnovers.

I will highlight some of the worst coding that I have seen so far:

1) Loop to nowhere

while (true){
   .........
}

If you want to write a process that never ends, don’t do this. I rather you use upstart to daemonize the script instead. Loops to nowhere is no different to the dreaded GOTO

2) Mixing linux commands with app script

$check = exec("ps aux | grep somePhpProcess | wc -l");
if (empty($check))
   exec("/usr/sbin/php somePhpProcess.php")

This is one script that I see in cron job. Yes, CRON JOB! Why don’t you use monit instead? it is a lot cleaner, and I am not a fan of php exec(). I stay clear of it like leprosy.

3) Loop through array just to find one record

public static function getRecordMem($id) {
            $listAll = self::getListMem();
            foreach( $listAll as $record ) {
                if($record->getKeyword()==$id)
                return $record;
            }
        }

Hello, there is such a thing call sql query. Please use that! And don’t store the whole table records on memory. You are going to get latency issues.

4) the “If-Else” burger

if($i == 1){
   $j = 1;
else if ($i == 2){
   $j = 2;
else if ....
else $j=100;

Burgers make you fat. This script is totally redundant when a simple $j = $i is enough to do the trick for you.

5) Return of the Null

public Employee getByName(String name) {
  int id = database.find(name);
  if (id) {
    return null;
  }
  return new Employee(id);
}

I know Star Wars is back but the Force is not with the null. Return Null in functions is bad idea. Don’t even think about it.

So in conclusion, do the industry a favour. Start doing clean code, and think through how to optimally deploy your app.

Advertisements

9 thoughts on “Top 5 worst coding practices

  1. What does using an external tool have to do with loops? If I want a program to have some thread running and doing something while the main program runs, then I can simply make it a daemon thread, put such a loop there, add a sleep at the end and be done with it.

    Or, even more simple, if I want a program running all the time (without having to restart it again and again and again and again, spending more time on initialization than on actual work) and do something, for example check an external service, this would be an easy easy way (if you don’t want to use a Timer, for example).

    I’m open for arguments why this should be considered a bad idea though…

    • the problem is the intention of the previous script writer wants to the script to run like a daemon, to infinity, hence the while(true) loop in 1) and a cron job to check if it is running in 2)

      I have no problems with while loop, but with a condition to break out from it. While(true) makes it no different from GOTO

  2. I also disagree with the null idea. Sorry, no, Employee.NOBODY is just a complicated way of saying “null”. Why should I return an employee if there isn’t such an Employee? Then null is the perfectly valid answer. No employee. And not “Well, you get an Employee, but actually you must now test if that is actually an Employee and not just an Employee Dummy object.”

    • If you read the article further, you will see this:

      – Hello, is it a software department?
      – Yes.
      – Let me talk to your employee “Jeffrey” please.
      – Hold the line please…
      – Hello.
      – Are you NULL?

      This explains it. I rather code the NOBODY way even though it may be harder, cos I care

      • I have read it and it makes a really bad argument, because “Are you NOBODY?” or “Are you really an employee or just a dummy object that looks like an employee?” makes the talk not really any better. In some cases there might be a useful default value, but that default value can depend on the situation, etc. Null gives you exactly one information “No employee”, while an employee that actually isn’t an employee because it doesn’t exist, does not. And I care, too, clean code is very important, but as of yet I have not found an argument that null is ALWAYS bad (for collections, etc. I totally agree, even for some other circumstances, sure – but not always).

    • Saw your last comment, and i cant reply as there is a limit how much nested reply is allowed. Lets keep an open mind. Would you mind show us specific examples when return null is ok and when it is not? I do not have a good impression of return null as it always create null pointer exceptions

      • Well, the simple Employee example, for example…

        Employee findEmployeeWithId(int employeeId);

        Now let’s imagine we ask for an Employee with a valid id but not assigned id (invalid id, for example < 0 would be, of course, a good reason for an exception). So we ask for the Employee with the id 1234, but no such Employee does exist.

        What can we do now?

        We did not ask for a Collection, so we cannot return an empty one (and I don't like returning Collections for 1:1 questions).

        We could return a Dummy Employee, for a example, even an "UnknownEmployee extends Employee". But then I would have to ask that Employee, if it actually an Employee, which doesn't seem to be better than null, which has the same information. Of course, for some specific uses, a DummyEmployee might be usable, for example one that returns wage zero, so that I can calculate, etc. – but these default values can vary from question to question and I still end up with the problem that I have an Employee-Object that isn't an Employee.

        Of course there are also more complicated ways, for example returning a FindResult, etc. – but we start violating KISS here. The code becomes complicated just to follow the dogma of "No Null" and we still must ask the question "Does this result point to an existing Employee?" sooner or later.

        Or we could throw an EmployeeDoesNotExistException. But sorry, this is not an error. It's totally normal. We asked for an Employee and it does not exist, fine. That itself is not an Exception, unless we have to assume that such an Employee actually SHOULD exist. And of course, throwing an Exception will create Overhead, will make our method calls more complicated, etc.etc.

        So, that leaves us with null. And personally, I think that's perfectly reasonable. "Give me the Employee…" "Null = Does not exist". I cannot simply see the problem here. Yes, of course you could solve it in other ways, but I think this is the only truly generic way of doing it in these situations. For everything else you need complicated workaround to convey the same result. In the end, the coder must understand the code and Null is the most simple solution here.

        (And I do not think that Null objects are the problems – bad code is. If you used DummyObjects instead of Nulls, we had similar errors because people would treat an UnknownEmployee as a real Employee, which simply won't cut it all the time.)

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s