Anti-patterns

In software engineering, an anti-pattern is a pattern that seems to work but is counter-productive and far from optimal in practice. An anti-pattern can easily result in unmaintainable and error-prone solutions. Usually, anti-patterns emerge over a period of time when new functionality is added incrementally without a focus on continuous refactoring. Maintenance of code by different developers over a period of time often compounds the problem.

Anti-patterns and languages supported

Here is the list of anti-patterns and the languages they are detected in:

God class (also called Monster Class) is a class that single-handedly implements large blocks of functionality within a system and performs multiple, non-cohesive functionalities. It violates the basic idea of structured programming which is: Break a big problem into many smaller problems, solve those, and the combination of results obtained inevitably solves the big problem. A God class contains much overall functionality in a single object, holds too much data and has too many methods. God classes are referred to as being the object-oriented analogy of failing to use subroutines in procedural programming languages.

Impact

• A God class tends to become bulky and complex.
• Refactoring becomes a fairly complicated task. Refactoring of such a class affects a significant part of the system, as other members of the system naturally depend heavily on God classes.
• The tendency to hold too much non-cohesive functionality also means this class becomes critical; making even a small change to it has a system-wide impact in terms of testing efforts.
• Time and maintenance cost increases.

Characteristics

• The class is large and complex (high LOC and too many long and complex methods).
• Methods implement non-cohesive functionalities.

public class BankAccount {
  private AccountInfo acInfo;
  // Cohesive methods
  public void openAccount();
  public void closeAccount();
  // Non-cohesive method (should ideally exist in Customer class)
  public void createCustomer();
...

• Class excessively accesses data of other classes and also tends to implement logic that belongs to those classes.

//Class BankAccount accesses attributes of AccountInfo class 
// directly private AccountInfo acInfo;
// Class also implements logic that should belong to AccountInfo class.
public boolean isAccountValid() {
  if( (acInfo.isActive) && (acInfo.balance > acInfo.minimum balance ) {
    return true;
  }
  return false;
}

• Solution: Move the logic into a method in AccountInfo class and invoke that method.

public boolean isAccountValid() {
  return acInfo.isAccountValid();
}

Example(s)

Statistics of Jedit

• The class has 143 methods and 31 attributes.
• The class accesses (directly or by getter/setter methods) about 28 attributes from 13 different classes – a characteristic of God Class.
• Methods seem to be long and complex.
• Methods have many branches (Total of 447 branches).

Inferred from the diagram above

• Public Interface and Internal Implementation layers:
• The class has too many methods (143). Methods seem to be complex.
• Attributes layer:
• Too many attributes (33)
• None of the class methods are accessing any of the attributes.
• This is a symptom of lack of sharing of common class level parameters.

Guidelines

• Follow the ‘one class one task’ rule. Distribute the system intelligence across several classes and let one class handle only one task. Collectively they will solve the intended problem.
• Functionalities that have common, overlapping aspects but are also dissimilar in other aspects can be implemented as sibling classes derived from a common base class.
• If distinct groups of data-operations exist in one class, try extracting them into separate classes.
• Make sure that class methods are not too long and do not use more than necessary data from other classes.
• God class is often a result of incrementally adding functionality to an existing class over time. This should be avoided. Try refactoring the class occasionally. Split the class instead of adding new functionality.

This anti-pattern applies to methods or classes with a high incoming coupling. Many different modules/components call into one specific module A and are dependent on it. Module A is said to be affected by the Shotgun Surgery anti-pattern.

Impact

• Many design entities (classes, modules) depend on the classes/methods affected by this anti-pattern. If a change occurs in this operation, all dependent classes and operations need to be fixed.
• Managing all changes becomes a significant task. Maintenance becomes expensive.
• If an affected method is large and complex (Brain method), refactoring becomes a complex task.
• Testability is affected. Even a minute change necessitates testing all caller functionality.

Characteristics

• The affected operation is called by too many other operations. This can be measured by the number of times the operation is called.
• Incoming calls are from many classes (This can be measured by counting the number of calling classes).

Example(s)

• Method ‘formatParagraph( )’ of class ‘TextArea’ is accessed by 10 different methods from 6 classes.
• Any change to this method would affect several dependent methods.

Guidelines

• Since a lot of functions call into a method affected by Shotgun Surgery, changing this method affects all calling methods. Move functionality (that can logically belong to Shotgun Surgery affected methods) from all client classes into the actual Shotgun Surgery method. This will keep all affected code in one place and also provide better encapsulation. This would also mean changes in the affected method would have minimum impact on callers of this method.
• If Shotgun Surgery methods are too large and complex, they could most likely be Brain Methods. Try to split such methods into shorter and less complex methods.

In object-oriented design, generally, data should be packaged together with the processes that use that data. The Feature Envy anti-pattern is one where a method seems more interested in a class other than the one it actually is in. The most common focus of envy is the data. The fundamental rule of thumb is to put things together that change together. Data and the behavior that references that data usually change together.

Impact

• Reduced maintainability.
• Envied data is likely to be accessed by several external classes. This makes the data critical.
• Methods envying the data can become unnecessarily large.
• Failure to keep data and functionality together is a violation of encapsulation. The envied class can tend to act like a Data Class simply providing access to its data.
• Distribution of system’s intelligence tends to be imbalanced.
• The Feature envy method possibly also uses data from its own class in addition to envied data. This means refactoring can become complex as the functionality now is a combination of those that belong to separate classes. It becomes hard to simply move this method over to the envied class.

Characteristics

• The method makes excessive use of attributes of other classes.
• The method uses far more attributes of other classes than its own.
• It uses those foreign attributes to implement functionality that should ideally, belong to foreign classes.
• The envied data accessed by methods affected by this anti-pattern belong to very few other classes.

Example(s)

• Method M1 ( ) ofTextArea application is affected by Feature Envy.
• TextArea has M1, M2, and M3 as member variables.
• The method Method 1 ( ) doesn’t use M1, M2, and M3 but uses members from the following Foreign Data Provider 4 members from FDP 2 3 members from FDP 1 1 members from FDP 3 7 members from FDP 4
• Thereby it’s more interested in other classes data than its own class.

Guidelines

• Methods shall access functionality of other classes rather than accessing their attributes directly.
• If a Feature Envy affected method is not accessing any of its own data, move it to the envied class.
• If a Feature Envy affected method is making use of its own data partially then break it down and move the envied functionality into the envied class. This will also balance the distribution of system intelligence.
• If a Feature Envy affected method is accessing data from two or more classes, move it to the class that is envied the most. After that point, break the functionality such that the part that belongs to another envied class is moved into that class.

A Global Hub is a component that has both, a high number of global dependents and is also globally dependent on too many components. In other words, it is both a global butterfly and a global breakable. Thus, it has a high number of both global incoming dependencies as well as global outgoing dependencies. The presence of global hubs results in high maintenance issues in the system.

Impact

• Global Hubs break the reusability of modules and make the code difficult to understand and to maintain.

Characteristics

• A Global Hub is directly or indirectly dependent on many other classes.
• A Global Hub object is used by too many other objects.
• Dependencies can be in the form of method calls, member access and local variable access.

Example(s)

• Global Hub has both incoming and outgoing dependencies.
• C2, C3, C5, C7 are incoming dependencies and C4, C1, C21 are outgoing dependencies.

Guidelines

Please refer to the guidelines of Global Butterfly and Global Breakable. Both have to be solved in order to fix the Global Butterfly issue.

The Local Breakable component has many immediate outgoing dependencies. Such a component carries excessive responsibility and is typically identified by many long methods. It represents too many local outgoing dependencies. The analogy is similar to Local Butterfly in the case of dependencies. The main difference here is that, there are local outgoing dependencies as instead of local incoming dependencies in Local Butterfly.

Impact

• Breakables make the code difficult to understand, to maintain and to reuse.

Characteristics

• The class is directly dependent upon too many other classes.
• The dependencies can be a member access or a local variable access to other classes.

Example(s)

• TextArea has 17 dependencies.
• C1 to C16 are direct dependencies.

Guidelines

• Try to remove direct dependencies on other classes by moving relevant methods and member variables to the class where it belongs.
• Extract new classes which logically group member variables and methods acting upon them.

The Global Breakable is a structural anti-pattern for a system component that is often affected when any other component is changed. It has too many global outgoing dependencies (transitive dependencies). The analogy is similar to Global Butterfly in the case of dependencies. The main difference is that here there are global outgoing dependencies as opposite to global incoming dependencies.

Impact

• Global breakables are undesirable because they indicate fragility and lack of modularity in the system.

Characteristics

• A class is directly or indirectly dependent on many other classes.
• Dependencies can be in the form of method calls, member access, and local variable access.
• It reduces maintainability.

Example(s)

• Any changes in the TextArea can break the TextArea.
• C1 to C8 and C17 are directly dependent on TextArea while C9 to C16 and C18 to C29 are indirectly dependent on the TextArea.

Guidelines

• Try to remove direct dependencies on other classes by moving relevant methods and member variables to the class where it belongs.
• Try to avoid Local Breakables and classes with many outgoing dependencies wherever possible, since they support the occurrence of Global Breakables.
• If this does not help, an overall review of the module architecture might be required.

When methods of a class access (depend on) many operations across an excessive number of classes, it is said that the class is dispersedly coupled with those provider classes.

Impact

• A change in a dispersively coupled method leads to changes in all the dependent (coupled) classes.

Characteristics

• Methods of affected classes call too many methods from other classes
• Communication with each of the external classes (that the affected class is coupled with) is not intense. The affected method calls one or few methods from each class it is coupled with.
• Calling methods have the non-trivial level of nested conditionals (nested IF-ELSE statements).

Example(s)

• Method A is the caller class is calling Method 1 from Service 1, Method 2 Service 2, Method 3 Service 3 and so on.

Guidelines

• Avoid calling methods from unrelated classes.
• In case of Intensive Coupling, multiple calls from the provider class can be combined into a smarter service that lives in the provider class and internally forwards calls to individual operations.

public class BankAccount {
  private AccountInfo acInfo;

  public Boolean isBankAccountValid()
    if(acInfo.condition1() == true) && (acInfo.condition2()==true)
    (acInfo.condition3() == true ) && ( acInfo.condition4() == true ) {
    // execute some code......
    }
  }
}

• Change the above implementation to:

public Boolean isBankAccountValid {
  return acInfo.isValid();
}

• In case of Dispersed Coupling (one method calling into methods of too many other classes) it is likely that the calling method is a Brain Method that does too many non-cohesive tasks. Try to split such a method.

Brain method is a method that tends to centralize the functionality of its owner class (very much in a way a God Class tends to centralize a system’s intelligence). It tends to be long as it carries out most of the tasks of the class that are supposed to be distributed among several methods. It has too many conditional branches and deep nesting.

Impact

• Brain methods are excessively long and complex. This impacts understandability and reusability.
• Too many conditional branches and too deep nesting necessitates excessive testing.
• Long methods often try to do several things in one place. This results in usage of excessive temporary variables. This makes the method error-prone.

Characteristics

• Method is excessively large(high LOC).
• Method has too many conditional branches (High Cyclomatic Complexity).

if (condition1 is TRUE){...... }
  else if (condition2 is TRUE){...... }
  else if (condition3 is TRUE){...... }
  else if (condition4 is TRUE){...... }
  // several more elseifs...
else {......}

• Method has deep nesting level (deeply nested IF-ELSE statements)

if (condition1 is TRUE){
  if (condition2 is TRUE){
    if (condition3 is TRUE){
      if (condition4 is TRUE){
        // execute something...
    }
  }
}

• Method uses too many variables (including local variables, data members, and global variables) – more variables than a human can keep in short-term memory.

Example(s)

public int getFoldLevel(int line)
  {
    if(line < 0 || line >= lineMgr.getLineCount())
      throw new ArrayIndexOutOfBoundsException(line);

    if(foldHandler instanceof DummyFoldHandler)
      return 0;

    int firstInvalidFoldLevel = lineMgr.getFirstInvalidFoldLevel();
    if(firstInvalidFoldLevel == -1 || line < firstInvalidFoldLevel)
    {
      return lineMgr.getFoldLevel(line);
    }
    else
    {
      if(Debug.FOLD_DEBUG)
        Log.log(Log.DEBUG,this,"Invalid fold levels from " + firstInvalidFoldLevel + " to " + line);

      int newFoldLevel = 0;
      boolean changed = false;
      int firstUpdatedFoldLevel = firstInvalidFoldLevel;

      for(int i = firstInvalidFoldLevel; i <= line; i++)
      {
        Segment seg = new Segment();
        newFoldLevel = foldHandler.getFoldLevel(this,i,seg);
        if(newFoldLevel != lineMgr.getFoldLevel(i))
        {
          if(Debug.FOLD_DEBUG)
            Log.log(Log.DEBUG,this,i + " fold level changed");
          changed = true;
          // Update preceding fold levels if necessary
          if (i == firstInvalidFoldLevel)
          {
            List precedingFoldLevels =
              foldHandler.getPrecedingFoldLevels(
                this,i,seg,newFoldLevel);
            if (precedingFoldLevels != null)
            {
              int j = i;
              for (Integer foldLevel: precedingFoldLevels)
              {
                j--;
                lineMgr.setFoldLevel(j,foldLevel.intValue());
              }
              if (j < firstUpdatedFoldLevel)
                firstUpdatedFoldLevel = j;
            }
          }
        }
        lineMgr.setFoldLevel(i,newFoldLevel);
      }

      if(line == lineMgr.getLineCount() - 1)
        lineMgr.setFirstInvalidFoldLevel(-1);
      else
        lineMgr.setFirstInvalidFoldLevel(line + 1);

      if(changed)
      {
        if(Debug.FOLD_DEBUG)
          Log.log(Log.DEBUG,this,"fold level changed: " + firstUpdatedFoldLevel + ',' + line);
        fireFoldLevelChanged(firstUpdatedFoldLevel,line);
      }

      return newFoldLevel;
    }
  }

Guidelines

• Split a brain method into two or more separate methods. This does not mean breaking down the method into several methods that are called in continuation. This simply increases level of nesting. Rather extract pieces of code into separate methods and invoke these methods in parallel from the original method.

Wrong way to split function_A( ):

function_A( ) {
  // execute few lines of code and then invoke function_B( ) 
  function_B( );
}

function_B( ) {
  // execute few lines of code and then invoke function_C( )
  function_C( );
}

function_C( ) {
  // execute few lines of code and then invoke function_D( ) 
  function_D( );
}

Recommended way to split function_A( )

function_A {
...... // execute some code, then invoke function_B( )
function_B( );
...... // execute some code, then invoke function_C( )
function_C( );
...... // execute some code , then invoke function_D( )
function_D( );
}

• If a method has significant pieces of code executed via too many conditional branches, use polymorphisms i.e., refactor class design in such a way that the code in conditional branches is delegated to overloaded/overridden functionality in children classes.
• During testing, make sure there is a test case for every independent path.
• Avoid usage of Global variables.
• Don’t use too many variables in a single method.

This anti-pattern is on the lines of Global Butterfly but differs in its scope of dependents. As per IBMSP, Local Butterfly is a structural pattern for a component that has many immediate dependents. The scope of dependents is limited to immediate dependents that directly access, use, reference, or contain the Local Butterfly.

Impact

• The impact is similar to that of Global Butterfly but limited in scope.
• A small change in the Local Butterfly impacts all immediate dependent objects.
• Testability and maintainability are affected.

Characteristics

• Too many objects directly depend on a Local Butterfly. Similar to a Global Butterfly, its dependence can be in form of access, reference, call, contained, instantiation, implements, extends, objects thrown (exceptions). Only direct/immediate dependents are considered.

Example(s)

• Using the tool Structural Analysis for Java to analyze TextArea application revealed that about 18 percent of objects in TextArea application are Local Butterflies.
• TextArea is represented as a Pink block at the center.
• TextArea has 19 immediate dependents.
• The rectangle represents objects that are immediate dependents of TextArea.
• Continuous arrows emerging from TextArea indicate that objects at the other end either contain, references, calls, or extends SimpleNode object.
• Dashed arrows emerging from TextArea indicate that object at the other end use TextArea object.
• Any changes in the TextArea can break these classes.

Guidelines

• Only basic Interfaces or utility classes should be allowed to become Local Butterflies.
• Avoid a design that deploys a low degree of Coupling (low value for the KPI – Coupling Between Objects) for each class.
• Group classes with related / similar functionality into common hierarchy (change Has-A relationship to Is-A relationship).

When methods of a class excessively access operations of one or a few other classes, it is said that the class is intensively coupled with those one or few provider classes.

Impact

• Understanding the relation between the client method & the classes providing services becomes difficult.

Characteristics

• Methods of affected class call too many methods. These methods are called from only a few other classes.
• Calling Methods have a non-trivial level of nested conditionals (nested IF-ELSE statements).

Example(s)

• The method formatParagraph in TextArea accesses 9 different methods from JEditBuffer.

Guidelines

• Moving intensively coupled operations to a single new operation.
• Split the method affected by Intensive Coupling into one or more methods based on the classes with which it is interacting.
• Reduce the number of outgoing interactions of this method with methods belonging to other classes by splitting the method and logically moving it to other classes.

Brain Class is a class that is complex partly because it implements a significant amount of system intelligence and partly because it holds one or more Brain methods.

Impact

• Low cohesion impacts understandability, maintainability and testability.
• Since a Brain Class becomes less cohesive or non-cohesive, refactoring becomes painful. Moreover, refactoring of Brain methods which are large and complex becomes even more complicated.

Characteristics

• The class is very large and complex (high LOC and has complex methods).
• The class has either:
  • One Brain method and size of all methods is very large, Or,
  • Has more than one Brain Methods, and methods are large and functional complexity is very high.
• The class is non-cohesive.
• The class is not a God class because it does “not” access foreign data significantly.

Example(s)

Analysis of class TokenMaker indicates:

• TokenMaker has 14 methods and 12 attributes.
• A Brain Method: A represents public method markTokens( )
• B Brain Method: B represents private method handleRule( )
• C Brain Method: C represents private method markKeyword( )
• Additionally: A, B, and C methods are represented as lengthy blocks indicating that their individual number of lines of code is significantly high.

Guidelines

• Split a non cohesive class into two or more cohesive classes. Follow the ‘one class one task’ rule.
• When creating class methods, avoid creating large and complex methods.
• Try refactoring Brain methods by splitting them into multiple methods (refer to guidelines of ‘Brain Method’ on how to split Brain methods).
• Brain methods tend to use significant duplicate code. Extract duplicate code into separate methods.
• Brain methods are often a result of incremental additions of functionality to existing methods. When adding new code to an existing method, keep LOC within limit. If not, create a new method
• In some cases a Brain class may be harmless, especially if proved over time that it has had no maintenance problems and is stable. In such a case avoid the costly effort of refactoring such a class.

As per IBMSP, Global Butterfly is a structural pattern for an object that has many global dependents. Changes to a global butterfly have a significant impact on the rest of the system. Global dependents include both direct (immediate) as well as indirect dependents. Applies to: Classes, Inner classes, Interfaces, Packages. All relationships are considered – Objects and interfaces accessed, referenced, called, contained, instantiated, implemented, thrown (exceptions).

Impact

• Since too many components are dependent on a Global Butterfly, a small change in this object has a system wide impact.
• Replacing a global butterfly component independently is almost impossible.
• System becomes fragile and non-modular. Refactoring becomes a very complex task.
• Testability and maintainability are affected.

Characteristics

• A Global Butterfly object is used by too many other objects. Usage can be in form of objects and interfaces accessed, referenced, called by, contained in, instantiated by, implements, extends, objects thrown (exceptions) than a human can keep in short term memory.

Example(s)

• TextArea is represented as a Pink block at the center.
• TextArea has 30 dependents
• The rectangle in blue represents objects that are immediate dependents of TextArea ( a direct reference to of TextArea object)
• The clusters of objects in yellow represent objects dependent on direct dependents of TextArea (inner rectangle objects).
• A continuous arrow emerging from TextArea indicates that an object at the arrow end directly depends (contains) TextArea object.
• A dashed arrow emerging from TextArea indicates that an object at the arrow end is using TextArea object an object from one of the outer clusters calling this dependent object (indirect dependency on TextArea)
• The directly depending are C3, C1, C6, C2 etc and the indirect dependencies are C11, C14, C26, C18, C20 etc.

Guidelines

• Only basic system interfaces or utility classes should be allowed to become Global Butterflies. Interfaces exist for the purpose of being extended by other classes and are acceptable Global Butterfly candidates that cause no harm to the system.
• Promote a design that deploys a low degree of Coupling (low value for the KPI – Coupling Between Objects) for each class.
• Group classes with related/similar functionality into a common hierarchy (change Has-A relationship to Is-A relationship).

This anti-pattern applies to classes or modules which have the implementation spread across multiple files.

Impact

• As the implementation is spread across multiple files, these modules/ classes tend to be large and often violate the single responsibility principle.
• The module/ class often grows over time and ends up becoming a central module/ class (or even a God class).

Characteristics

• The implementation is spread across multiple files.
• In general, multiple files are added over the history of the module/ class instead of creating a new module/ class for a new responsibility.

Guidelines

• Since the module/ class is already split into multiple files, it should be seen if those files point to different responsibilities of the module/ class.
• Often these modules/ classes point to missing abstractions. If such abstractions exist, they should be extracted out.

This anti-pattern applies to modules/classes which have a large interface with too many exposed functions/methods

Impact

• As the interface of these modules/classes is large, it violates the interface segregation principle (no client should be forced to depend on methods it does not use).
• Often these modules/classes have many incoming dependencies with multiple clients using the provided functionality.
• Any bug in these modules/classes has a wide impact on the system.

Characteristics

• The modules/classes have too many exposed methods.

Guidelines

• The module/ class should be considered for segregation by forming logical groups of exposed functions/ methods.
• If there are any functions/ methods which should not be exposed, they should be made private.

Long sequence of outgoing method/function calls detected.

Impact

• Message Chains are undesirable because they indicate fragility as intermediate dependencies are dependencies in disguise.

Characteristics

• A function/method calls many functions which internally calls many other functions.

Guidelines

• It can be beneficial to move a method to a class that contains most of the data used by the method.

This anti-pattern applies to functions/methods which have high direct outgoing dependencies.

Impact

• Fan outs are undesirable as changes to the signature of called functions/methods require a change in the caller fan out function/method.
• Fan outs indicate fragility.

Characteristics

• A function/method directly calls many functions.

Guidelines

• Try to remove direct dependencies on other functions or methods by moving them to the class where they belong.

This anti-pattern applies to functions/ methods which are not called from external modules/ components but are still exposed.

Impact

• It unnecessarily exposes the interface to client components/ modules
• If the function/ method is intended to be used privately, calling such functions/ methods might have undesirable output.

Characteristics

• The function/ method is not called from external modules/ components.
• The function/ method is called from within the own module/ component.

Guidelines

• The functions/ methods should not be exposed to other components (depending on the programming language they should be marked as private).

This anti-pattern applies to functions/methods which are not called.

Impact

• Unused methods unnecessarily bloat the code.
• Such methods adversely impact understandability and maintainability.

Characteristics

• A function/ method is not called.

Guidelines

• Consider removing or commenting the unused methods.

Structs defined in header expose the internals. They should be moved to .c source files.

Impact

• Clients can alter the internals of the data structure themselves leading to costly changes of all clients. Leads to very tight coupling.

Characteristics

• Definition of struct is present in header files which are exposed to clients.

Example(s)

• In this example details of Customer struct is exposed directly in header file. Clients can alter order quantity, or can use any attribute directly inside their code leading to very tight coupling.

// file: Customer.h
/* Include guards and include files omitted. */
#define MAX_NO_OF_ORDERS 42

/* Internal representation of a customer. */
typedef struct
{
  const char* name;
  Address address;
  size_t noOfOrders;
  Order orders[MAX_NO_OF_ORDERS];
}

Customer;
void initCustomer(Customer* theCustomer, const char* name, const Address* address);
void placeOrder(Customer* customer, const Order* order);

/* A lot of other related functions... */

Guidelines

• Structs that are defined in the header and expose the internals should be moved to .c source files.

Variables should not be exposed in the header files. Proper encapsulation of a module requires data hiding. All internal data should only be in private variables inside the .c/ .cpp source code files.

Impact

Data is visible to clients and they can alter its contents. This may lead to incorrect functionality.

Characteristics

Global variables defined in header files which are exposed to clients.

Example(s)

• Here DEFAULT_QUUX is exposed to b.c file also which is not intended.

// header.h
extern int DEFAULT_QUUX; // Data exposure

/* some other function declarations */

// a.c
#include "header.h"
int DEFAULT_QUUX = 5;
int main() {
  Blip b;
  b.do_stuff(); // makes DEFAULT_QUUX = 6
  Qlip q;
  q.do_things(); // reads DEFAULT_QUUX
}

// b.c
#include "header.h"
int fun() {
  return 0;
}

The global variable anti-pattern is detected for global non-static and non-const variables which are exposed using “extern” keyword. This anti-pattern is only applicable to C/C++.

Impact

• As Global variables can be read and modified from multiple parts of the program, it is difficult to restrict access making them prone to bugs.
• Global variables add implicit coupling between the variables and user functions/ modules thus impacting cohesiveness.
• If multiple threads access the same Global variable, it might lead to concurrency issues if synchronization is not handled correctly.
• Source code entities are easiest to understand when their scope is limited. Global variables can be read and modified from multiple parts of the program making it difficult to understand and maintain.

Characteristics

• The variable is a Globally exposed variable
• The variable is non-static and non-const
• The variable is exposed using “extern” keyword.

Example(s)

// header.h
/ file1.c
int g_intVar = 10; // Define global variable “g_intVar”
 
// file2.c
extern int g_intVar; // Used global variable defined in file1.c
 
int fun_a(){
if(g_intVar < 20){
g_intVar = 20; // Modified global variable
}
}
 
// file3.c
extern int g_intVar;
 
// Function takes int parameter as reference
int fun_b(int & outVar) {
 // Some condition
 ….
 outVar = 50;
 return 0;
}
 
int fun_c(){
 int ret = fun_b(g_intVar); // Passing global variable leading to its modifications
}

Guidelines

• Pass the variable as a parameter instead of using the Global variable.
• Use getter and setter functions to get and set the variable instead of making it global. This will also help in adding more constraints when the setter is called.
• If the variable is not expected to be changed, make it “const”.

Test hungry methods demand a high amount of unit/integration tests in order to cover all the execution paths, and this is especially important given the high usage of these methods.

Impact

Since many other methods or functions are dependent on test hungry methods, a change to this method will have an extensive impact.

Characteristics

• The method / function has high complexity.
• The method / function has high incoming dependencies.

Guidelines

• All statements and branches in a test hungry method should be covered in unit testing.
• If a a test hungry method has too many conditional branches, consider refactoring it into multiple small methods / functions.

Monster file single-handedly implements large blocks of functionality within a system and performs multiple, non-cohesive functionalities. It violates the basic approach of structured programming which is: Break a big problem into smaller problems and then resolve these problems. The result obtained inevitably solves the big problem. The monster file typically has high LOC, high LCOM, high CC, and a higher count of functions.

Impact

• The Monster file tends to become bulky and complex.
• Refactoring becomes a fairly complicated task. Refactoring of such a file affects a significant part of the system, as other members of the system depend heavily on Monster File.
• The tendency to hold too much non-cohesive functionality also means this file becomes critical; making even a small change to it has a system-wide impact in terms of testing efforts.
• Time and maintenance cost increases.

Characteristics

• The file is large and complex (high LOC and many long and complex functions).
• Functions implement non-cohesive functionalities.

Example(s)

• The file is large and complex (high LOC and too many long and complex functions).

Functions implement non-cohesive functionalities.

BankAccount.js {
let acInfo = [];
// Cohesive methods
public void openAccount();
public void closeAccount();
// Non-cohesive method (should ideally exist in Customer class)
public void createCustomer();

File excessively accesses data of other files and also tends to implement logic that belongs to those files.

//File BankAccount accesses attributes of AccountInfo file
// File also implements logic that should belong to AccountInfo file.
public boolean isAccountValid{ 
  if( (acInfo.isActive) && (acInfo.balance > acInfo.minimumBalance ) { 
    return true; 
  } return false; 
}

Solution: Move the logic into a method in the AccountInfo file and invoke that function.

public boolean isAccountValid() {
return acInfo.isAccountValid();
}

Guidelines

• Follow the ‘one file one task’ rule. Distribute the system intelligence across several files and let one file handle only one task. Collectively they will solve the intended problem.
• Functionalities that have common, overlapping aspects but are also dissimilar in other aspects can be implemented as sibling files and the common functionality can be moved into a third file.
• If distinct groups of data-operations exist in one file, try extracting them into separate files.
• Make sure that file functions are not too long and do not use more than necessary data from other files.
• The monster file is often a result of incrementally adding functionality to an existing file over time. This should be avoided. Try refactoring the file occasionally. Split the file instead of adding new functionality.

A function with nested inner functions and a depth of more than 3 levels is a Nested function.

Impact

• A heavily nested function makes it bulky and complex to read and maintain.
• Time and maintenance cost increases.
• Possibility of duplication of function increases.

Characteristics

• A function has multiple inner functions including anonymous functions and arrow functions.

function genModuleGetter (instanceId) {
var instance = instances[instanceId];
return function (name) {
var nativeModule = modules[name] || [];
var output = {};
var loop = function ( methodName ) {
Object.defineProperty(output, methodName, {
enumerable: true,
configurable: true,
get: function proxyGetter () {
return function () {
var args = [], len = arguments.length;
while ( len-- ) args[ len ] = arguments[ len ]; return instance.document.taskCenter.send('module', { module: name, method: methodName }, args)
}
},
set: function proxySetter (val) {
if (typeof val === 'function') {
return instance.document.taskCenter.send('module', { module: name, method: methodName }, [val])
}
}
});
};
for (var methodName in nativeModule) loop( methodName );
return output;
}
}

Solution: Move the logic of inner functions into separate independent functions in the same file.

function genModuleGetter (instanceId) {
var instance = instances[instanceId];
return getOutput(name, instance);
}
function getOutput(name, instance)
{
var nativeModule = modules[name] || [];
var output = {};
var loop = iterate( methodName , output, instance)
for (var methodName in nativeModule) loop( methodName );
return output
}
function iterate(methodName , output, instance)
{
Object.defineProperty(output, methodName, {
enumerable: true,
configurable: true,
get: proxyGetter (methodName, instance),
set: proxySetter (val, methodName, instance)
});
}
function proxyGetter (methodName, instance)
{
return function ()
{
var args = [], len = arguments.length;
while ( len-- ) args[ len ] = arguments[ len ];
return instance.document.taskCenter.send('module', { module: name, method: methodName }, args)
}
}
function proxySetter (val, methodName, instance)
{
if (typeof val === 'function') {
return instance.document.taskCenter.send('module', { module: name, method: methodName }, [val])
}
}

Guidelines

• Follow the ‘one function one task’ rule.
• Create separate named functions instead of inner anonymous functions. It makes functions more readable and maintainable.
• Declare functions with the intention of reusing the logic they offer.

A closure having nested closures with the depth of more than 2 levels is less readable, slower in performance and hold excessively large private memory causing memory leaks.

Impact

• A heavily nested closure is complex to read and maintain.
• Creating closures inside other closure leads to duplication in memory, potentially slowing down the application.

Characteristics

A function that returns an inner function where the inner function will have access to the outer scope even after the outer function is executed.

function foo() {
  var height = 10; 
  function bar() {
    console.log(height);
    function counter() {
    var area = 62;
    function privateCounter() {
      console.log(area);  
      function myFunc() {
        var square = 21;
        function funcA() {
        console.log(square);    
        function getMeasure() {
          var measure = 27;
          function setLogs() {
          console.log(measure);
          }
          return setLogs;
        }
        var test = getMeasure(); 
        test(); 
        }
        return funcA;
      }
      var secret = myFunc(); 
      secret(); 
       }
      return privateCounter;
    }
    var invoke = counter(); 
    invoke(); 
  }
  return bar;
}
var something = foo(); 
something();

Solution: Move the logic of inner closures into separate closures if they are independent of each other or if the inner closure does not require the lexical scope of the outer function.

function foo() {
  var height = 10;
  function bar() {
    console.log(height);
  }
  return bar;
}
var something = foo(); 
something(); 
function counter() {
  var area = 62;
  function privateCounter() {
    console.log(area);
  }
  return privateCounter;
}
var invoke = counter(); 
invoke(); 
function myFunc() {
  var square = 21;
  function funcA() {
    console.log(square);
  }
  return funcA;
}
var secret = myFunc(); 
secret(); 
function getMeasure() {
  var measure = 27;
  function setLogs() {
    console.log(measure);
  }
  return setLogs;
}
var test = getMeasure(); 
test();

Guidelines

• When the lexical scope of inner function is independent of the outer functions, closures are not required as closures consume unnecessary system resources.
• The closure impacts the lifetime of its private state. The private state will be garbage-collected only after the outermost closure function is collected.
• Unless you need data privacy, it is more practical to use the module pattern to create new objects with shared methods.

A promise is an object which can be returned synchronously from an asynchronous function. Every promise executor function should call resolve and reject function.

Impact

• If any of the executer functions are not called, promise will not be settled and remains in its pending state.
• Any .then() or .catch() listeners for resolve or reject transitions will never get called.
• Most code, that uses promises, expects them to resolve or reject at some point in the future. In that case, if they don’t, then that code generally never gets to serve its purpose.

Characteristics

It will be in one of the three possible states:

• Fulfilled: meaning the operation fulfilled successfully.
  onFulfilled() will be called (e.g. resolve() was called)
• Rejected: meaning that operation has failed.
  onRejected() will be called (e.g. reject() was called)
• Pending: This means operation is not yet fulfilled or rejected.

Example(s)

function promise() 
{
  return new Promise((resolve,reject)=>  
  {
    var result = isValid();                                                 
    if(result)     
     resolve();                                                                                                                          
   });  
}                                                                              

Solution: For each new promise, resolve and reject functions are called.

function promise() 
{  
 return new Promise((resolve,reject)=>
 {
   var result = isValid(); 
   if(result) 
    resolve();
   else
    reject();
  });
}

Guidelines

When the executor obtains the result, it should call these callbacks:

• resolve(value) — if the job finished successfully, with result value.
• reject(error) — if an error occurred, error is the error object.

Markup in Javascript is used to insert HTML elements in javascript.

Impact

• Creating HTML elements in JS will decrease the readability & maintainability of code affecting reusability as well.
• This will break the basic principle i.e. separation of presentation and business logic.
• This will also lead to high memory consumption on the browser, for example creating an HTML element using a jquery object is a heavy data object.
• This also can reduce the performance of page loading in the browser.

Example(s)

function addHTML() { 
    var wrapper= $('
', { class: 'wrapper' });
    var input = $('', { class: 'email', type:'text' });
    wrapper.append(input);
    $('body').html(wrapper);
}

Solution: Email template is embedded in an HTML function

function addHTML() 
{                                              
   $('body').html(Template.emailForm());                  
}                                                                                                                                      

emailForm.hbs

 
 
  
 This is a test class 
 
 

Guidelines

• Use templates such as handlebar, mustache, etc instead of generating HTML from Javascript.
• Javascript file should not contain HTML elements.
• HTML template and JS parameters should work separately.

Variable Hoisting : 

• Hoisting is a JavaScript mechanism where variables and function declarations are moved to the top of their scope before code execution. This means that no matter where functions and variables are declared, they are moved to the top of their scope regardless of whether their scope is global or local. 
• The variable declarations are dispersed throughout the function definition. Because of this, it is an antipattern.

Impact

• Variables declaration at multiple places in a single scope reduces the readability of code.
• Hence, this code is difficult to maintain.

Characteristics

• Variables are declared in all over the scope in a scattered manner.

Example(s)

function declare() 
{   
 var  a = 10; 
 console.log(a); // 10   
 console.log(b); // undefined                                     
 var b = 11; 
 console.log(b); // 11
}                                                            

Solution: All the variables are declared at the top of the function.

function declare() 
{  
 var  a = 10, b = 11;
 console.log(a);     // 10
 console.log(b);    // 11
}

Guidelines

1. Variable declaration should be at the top in the scope.
2. Fixing this will improve the readability and maintainability of the code.

A direct dependency is formed when two classes or files have dependency over each other. The dependency can be caused by using variables or function calls.

Impact

• Cyclic dependency creates tight coupling between these classes and it is hard to reuse as separate units.
• Sometimes, circular dependency may cause memory leaks by preventing garbage collector to do their work.

Example(s)

C1 and C2 are the components that are directly dependent on each other.

Guidelines

• Cyclic dependency can be removed by using design patterns like observer or simply introducing interfaces.
• The code can be refactored to separate out the common logic to make those 3 classes where 2 classes depend on a 3^rd class.

When one file is coupled or dependent on many other files, it is called as file coupling. The dependency can be in the form of variables or functions. This applies to only those file-level variables or functions.

Impact

• Higher coupling with other files makes the code fragile. This is because the changes in other files may impact the concerned file and its logic.

Characteristics

• The file is dependent on multiple other files making it heavily coupled.

Guidelines

• Try to refactor the code so that the file is dependent on fewer files.
• The threshold value is 10.