Code Smells

1) Bloasters

• methods and classes that have increased to such gargantuan proportions that they are hard to work with.
• these smells do not crop up right away, rather they accumulate over times as the program evolves

Long methods

• if you feel you need to comment on something in your method, you should take this code and put it in a new method.
• Even a single line can and should be split off in to a separate method, if it requires explanations.
• And if the method has a descriptive name, nobody will need to look at the code to see what it does.

Move a descriptive code to a separate new method and replace the old code with a call to the method.

// PROBLEM
void printOwing() {
  printBanner();

  // Print details.
  System.out.println("name: " + name);
  System.out.println("amount: " + getOutstanding());
}

// SOLUTION
void printOwing() {
  printBanner();
  printDetails(getOutstanding());	// Print details.
}

void printDetails(double outstanding) {
  System.out.println("name: " + name);
  System.out.println("amount: " + outstanding);
}

 

Move the entire expression to a separate method and return the result from it.

// PROBLEM
double calculateTotal() {
  double basePrice = quantity * itemPrice;
  if (basePrice > 1000) {
    return basePrice * 0.95;
  }
  else {
    return basePrice * 0.98;
  }
}

// SOLUTION
double calculateTotal() {
  if (basePrice() > 1000) {
    return basePrice() * 0.95;
  }
  else {
    return basePrice() * 0.98;
  }
}
double basePrice() {
  return quantity * itemPrice;
}

 

Long Parameter List

• Check what values are passed to parameters. If some of the arguments are just results of method call of another object, use replace Parameter with Method Call.
• Instead of passing a group of data received from another object as parameters, pass the object itself to the method, by using Preserve Whole Object.

// PROBLEM
int basePrice = quantity * itemPrice;
double seasonDiscount = this.getSeasonalDiscount();
double fees = this.getFees();
double finalPrice = discountedPrice(basePrice, seasonDiscount, fees);

// SOLUTION
int basePrice = quantity * itemPrice;
double finalPrice = discountedPrice(basePrice);

// PROBLEM
int low = daysTempRange.getLow();
int high = daysTempRange.getHigh();
boolean withinPlan = plan.withinRange(low, high);

// SOLUTION
boolean withinPlan = plan.withinRange(daysTempRange);

 

Primitive Obsession

• Creating a primitive field is so much eaiser than making a whole new class. "Just a field for storing some data!"
• If you have a large variety of primitive fields, it maybe possible to group some of them in to their own class.
• If the value of primitive fields are used in method parameters, go with Introduce Parameter Object or Preserve Whole Object.

Intoduce Parameter Object

// PROBLEM
int low = daysTempRange.getLow();
int high = daysTempRange.getHigh();
boolean withinPlan = plan.withinRange(low, high);

// SOLUTION
boolean withinPlan = plan.withinRange(daysTempRange);

 

---

2) Object-Orientation Abusers

• You have complex switch operator or sequence of if statements.
  • To isolate switch and put it in the right class, you mad need Extract Method and then Move Method.
• Temporary fields get their values only under certain circumstances. Outside of these circumstatnces, they're empty.

When one class does the work of two, awkwardness results. Create a new class and place the fields and methods responsible for the relevant functionality int it.

Extract Class

 

You have a in which the local variables are so intertwined that you can't apply Extract Method.

class Order {
  // ...
  public double price() {
    double primaryBasePrice;
    double secondaryBasePrice;
    double tertiaryBasePrice;
    // Perform long computation.
  }
}

Transform the method into a separate class so that the local variables become fields of the class. 

Then you can split the method into several methods within the same class.

class Order {
  // ...
  public double price() {
    return new PriceCalculator(this).compute();
  }
}

class PriceCalculator {
  private double primaryBasePrice;
  private double secondaryBasePrice;
  private double tertiaryBasePrice;
  
  public PriceCalculator(Order order) {
    // Copy relevant information from the
    // order object.
  }
  
  public double compute() {
    // Perform long computation.
  }
}