How does bad code appear?

Bad code doesn't just appear in a code base; programmers write bad code. Most of the time, bad code can be written because of any of the following reasons:

• Use of wrong approaches by developers when writing code that is often attributed to tight coupling of components
• Faulty program designs
• Bad naming conventions for program elements and objects
• Writing code that is not readable as well as having a code base without proper test cases, thus causing difficulty when there is a need to maintain the code base

Tight coupling

Most legacy software applications are known to be tightly coupled, with little or no flexibility and modularity. Tightly coupled software components lead to a rigid code base which can be difficult to modify, extend, and maintain. As most software applications evolve over time, big maintenance issues are created when components of applications are tightly coupled. This is due to the changes in requirements, user's business processes, and operations.

Third-party libraries and frameworks reduce development time and allow developers to concentrate on implementing users' business logic and requirements without having to waste valuable productive time reinventing the wheel through implementing common or mundane tasks. However, at times, developers tightly couple the applications with third-party libraries and frameworks, creating maintenance bottlenecks that require great efforts to fix when the need arises to replace a referenced library or framework.

The following code snippet shows an example of tight coupling with a third-party smpp library:

public void SendSMS()
{
    SmppManager smppManager= new SmppManager(); 
    smppManager.SendMessage("0802312345","Hello", "John");
}

public class SmppManager
{
    private string sourceAddress;
    private SmppClient smppClient;

    public SmppManager()
    {
       smppClient = new SmppClient();
       smppClient.Start();            
    }        
       
    public void SendMessage(string recipient, string message, string senderName)
    {
       // send message using referenced library            
    }    
}

Code smell

Code smell is a term that was first used by Kent Beck, which indicates deeper issues in the source code. Code smell in a code base can come from having replications in the source code, use of inconsistent or vague naming conventions and coding styles, creating methods with a long list of parameters, and having monster methods and classes, that is methods or classes that know and do too much thereby violating the single responsibility principle. The list goes on and on.

A common code smell in the source code is when a developer creates two or more methods that perform the same action with little or no variation or with program details or facts that ought to be implemented in a single point replicated in several methods or classes, leading to a code base that is not easy to maintain.

The following two ASP.NET MVC action methods have lines of code that create a strongly-typed list of strings of years and months. These lines of code, that could easily have been refactored into a third method and called by both methods, have been replicated in these two methods:

[HttpGet]
public ActionResult GetAllTransactions()
{
    List<string> years = new List<string>();
    for (int i = DateTime.Now.Year; i >= 2015; i--)
         years.Add(i.ToString());
    List<string> months = new List<string>();
    for (int j = 1; j <= 12; j++)
         months.Add(j.ToString());
    ViewBag.Transactions= GetTransactions(years,months);
     return View();
}


[HttpGet]
public ActionResult SearchTransactions()
{
    List<string> years = new List<string>();
    for (int i = DateTime.Now.Year; i >= 2015; i--)
         years.Add(i.ToString());
    List<string> months = new List<string>();
    for (int j = 1; j <= 12; j++)
        months.Add(j.ToString());
    ViewBag.Years = years;
    ViewBag.Months = months;
    return View();
}

Another common code smell occurs when developers create methods with a long list of parameters, as in the following method:

public void ProcessTransaction(string  username, string password, float transactionAmount, string transactionType, DateTime time, bool canProcess, bool retryOnfailure)
{
    //Do something
}

Bad or broken designs

Quite often, the structure or design and patterns used in implementing an application can result in bad code, most especially when object-oriented programming principles or design patterns are wrongly used. A common anti-pattern is spaghetti coding. It is common among developers with little grasp of object-orientation and this involves creating a code base with unclear structures, little or no reusability, and no relationships between objects and components. This leads to applications that are difficult to maintain and extend.

There is a common practice among inexperienced developers, which is the unnecessary or inappropriate use of design patterns in solving application complexity. The design patterns when used incorrectly can give a code base bad structure and design. The use of design patterns should simplify complexity and create readable and maintainable solutions to software problems. When a pattern is causing a readability issue and overtly adding complexity to a program, it is worth reconsidering whether to use the pattern at all, as the pattern is being misused.

For example, a singleton pattern is used to create a single instance to a resource. The design of a singleton class should have a private constructor with no arguments, a static variable with reference to the single instance of the resource, and a managed public means of referencing the static variable. A singleton pattern can simplify the access to a single-shared resource but can also cause a lot of problems when not implemented with thread safety in mind. Two or more threads can access the if (smtpGateway==null) line at the same time, which can create multiple instances of the resource if the line is evaluated to true, as with the implementation shown in the following code:

public class SMTPGateway
{
    private static SMTPGateway smtpGateway=null;

    private SMTPGateway()
    {
    }

    public static SMTPGateway SMTPGatewayObject
    {
        get
        {
            if (smtpGateway==null)
            {
                smtpGateway = new SMTPGateway();
            }
            return smtpGateway;
        }
    }
} 

Naming the program elements

Meaningful and descriptive element naming can greatly improve the source code's readability. It allows easy comprehension of the logical flow of the program. It is amazing how software developers still give names to program elements that are too short or not descriptive enough, such as giving a variable a letter name or using acronyms for variable naming.

Generic or elusive names for elements lead to ambiguity. For example, having a method name as Extract() or Calculate() at first glance results in subjective interpretations. The same is applicable to using vague names for variables. For example:

int x2;

string xxya;

While program element naming in itself is an art, names are to be selected to define the purposes as well as succinctly describe the elements and ensure that the chosen names comply with the standards and rules of the programming language being used.

More information on acceptable naming guidelines and conventions is available at: https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/naming-guidelines.

Source code readability

A good code base can be easily distinguished from a bad one by how quickly a new team member or even the programmer can easily understand it after leaving it for a few years. Quite often, because of tight schedules and approaching deadlines, software development teams tend to compromise and sacrifice professionalism to meet deadlines, by not following the recommended best practices and standards. This often leads them to produce code that is not readable.

The following code snippet will perform what it is intended to do, although it contains elements written using terrible naming conventions and this affects the code's readability:

public void updatetableloginentries()
{
   com.Connection = conn;
   SqlParameter par1 = new SqlParameter();
   par1.ParameterName = "@username";
   par1.Value = main.username;
   com.Parameters.Add(par1);
   SqlParameter par2 = new SqlParameter();
   par2.ParameterName = "@date";
   par2.Value = main.date;
   com.Parameters.Add(par2);
   SqlParameter par3 = new SqlParameter();
   par3.ParameterName = "@logintime";
   par3.Value = main.logintime;
   com.Parameters.Add(par3);
   SqlParameter par4 = new SqlParameter();
   par4.ParameterName = "@logouttime";
   par4.Value = DateTime.Now.ToShortTimeString(); ;
   com.Parameters.Add(par4);
   com.CommandType = CommandType.Text;
   com.CommandText = "update loginentries set logouttime=@logouttime where username=@username and date=@date and logintime=@logintime";
   openconn();
   com.ExecuteNonQuery();
   closeconn();
}

Poor source code documentation

Code can be easily understood when written using the programming language's coding style and convention while avoiding the bad code pitfalls discussed earlier. However, source code documentation is very valuable and its importance in software projects cannot be overemphasized. Brief and meaningful documentation of classes and methods can give developers a quick insight into their internal structures and operations.

Understanding a complex or poorly written class becomes a nightmare when there is no proper documentation in place. When the original programmer that wrote the code is no longer around to provide clarifications, valuable productive time can be lost trying to understand how the class or method is implemented.

Non-tested code

Though many articles have been written and discussions have been initiated at various developers' conferences on different types of testing—test-driven development, behavior-driven development, and acceptance test-driven development—it is very concerning that there are developers that continuously develop and ship software applications that are not thoroughly tested or tested at all.

Shipping applications that are poorly tested can have catastrophic consequences and maintenance problems. Notable is NASA's Mars Climate Orbiter launched on December 11, 1998 that failed just as the orbiter approached Mars, due to a software error caused by an error in conversion where the orbiter's program code was calculating a metric in pounds instead of newtons. A simple unit testing of the particular module responsible for calculating the metrics could have detected the error and maybe prevented the failure.

Also, according to the State of Test-First Methodologies 2016 Report, a survey of the adoption of test-first methodologies of more than 200 software organizations from 15 different countries, conducted by a testing services company named QASymphony, revealed that nearly half of the survey respondents had not implemented a test-first methodology in the applications they had developed.