Long Method

Description (What)

A method/function that has too many lines of code. A general rule of thumb is to check if any method longer than 10-20 lines is a long method.

A long method is also described as method that primarily violates the Single Responsibility principle.

How to Locate It (Where)

An easy way to find Long Methods is to look for any comments in the method. If there are comments, it usually means that the code explained by explicit comments can be made into its own method.

How It Manifests (Why)

Programmers keep adding onto the method instead of creating a new method because its easier.

How to Fix It (Possible Treatments)

Usually, conditional operators, loops and switch statements are a good indicators that code can be moved to a separate method.

For conditionals, use Decompose Conditional.
If loops are in the way, try Extract Method.
If long Switch statement, try Extract Interface plus HashMap Technique, to preserve the automaticity.

In both examples below, the treatment used is called Extract Method. In this treatment, we identify a part(s) of the method which can qualify as its own method under the single responsibility rule and remove it from the original method and make it its own separate method.

Other treatments are also possible based on the specific scenario if Extract Method cannot be applied, they can be found here

Examples

Example 1

Before:

The method boardFlight() does a lot of work that can make it confusing to understand and should be broken into smaller methods.

Observed Code Smells:
- Long Method (line 62-92)

After:

Applied Extract Methods to the method and created three new methods securityCheck, noFlyListCheck, passengerCanBoard.

Refactoring Applied:
- Long Method
    - Extract Method (securityCheck, noFlyListCheck, passengerCanBoard)

Observed Code Smells After Refactoring:
- None

Note: The if-else-if statements in boardAirplane() method are not considered a code smell because of the ignore case in Switch Statements code smell When a switch operator performs simple actions, there’s no reason to make code changes

Example 2

Before:

The class TextAnalyzerVariation is a utility class that contains a useful method to calculate information about a .txt file.

The 'analyze' method is now doing everything:

Calculate the word count,
Calculate the sentence count,
Calculate the average word length,
Identify the most common word.
which, makes the method extremely long.

Observed Code Smells:
- Long Method (lines 17-53, roughly)

After:

Applied Extract Method to take each calculation out as a separate method.

Refactoring Applied:
- Long Method
    - Extract Method (line 17-70, roughly)

Note: The line numbers might have a little discrepancies.

Observed Code Smells After Refactoring:
- None

When to Ignore

Not applicable in the given examples.

More about Long Method

Example 1
Example 2

Before
After

001  import java.util.ArrayList;
002  
003  class Passenger {
004      String name;
005      ArrayList<String> items;
006      String flightNumber;
007      boolean boarded;
008  
009      public Passenger(String name, String flightNumber) {
010          this.items = new ArrayList<String>();
011          this.name = name;
012          this.flightNumber = flightNumber;
013      }
014  
015      public void addItem(String item) {
016          items.add(item);
017      }
018  
019      public void removeItem(String itemToRemove) {
020          for (String item : this.items) {
021              if (item.equals(itemToRemove)) {
022                  this.items.remove(itemToRemove);
023              }
024          }
025      }
026  
027      public ArrayList<String> getItems() {
028          return this.items;
029      }
030  
031      public String getName() {
032          return this.name;
033      }
034  
035      public String getFlightNumber() {
036          return this.flightNumber;
037      }
038  
039      public void boardFlight() {
040          this.boarded = true;
041      }
042  
043      public boolean getBoardedStatus() {
044          return this.boarded;
045      }
046  }
047  
048  class AirportTerminal {
049  
050      String[] INVALIDITEMS;
051      String[] NOFLYLIST;
052      String[] CURRENTFLIGHTS;
053      Passenger passenger;
054  
055      public AirportTerminal(Passenger passenger) {
056          this.passenger = passenger;
057          this.INVALIDITEMS = new String[] { "Arms", "Knives", "Liquids" };
058          this.NOFLYLIST = new String[] { "Joseph Carn", "Mel Gibbs", "Hali Burton" };
059          this.CURRENTFLIGHTS = new String[] { "AC102", "BD309", "DL089" };
060      }
061  
062      public void boardAirplane() {
063  
064          for (String invalid : INVALIDITEMS) {
065              for (String item : passenger.getItems()) {
066                  if (item.equals(invalid)) {
067                      throw new Error("Invalid Item Detected");
068                  }
069              }
070          }
071          System.out.println("Finished Security Check");
072  
073          for (String noflyName : NOFLYLIST) {
074              if (noflyName.equals(this.passenger.getName())) {
075                  throw new Error("This passenger is on the No Fly List");
076              }
077          }
078          System.out.println("Passenger can proceed to their gate");
079  
080          for (String currentFlight : CURRENTFLIGHTS) {
081              if (currentFlight.equals(this.passenger.getFlightNumber())) {
082                  System.out.println("Flight has begun boarding");
083                  this.passenger.boardFlight();
084              }
085          }
086  
087          if (!this.passenger.getBoardedStatus()) {
088              throw new Error("Flight isn't ready to board");
089          }
090  
091      }
092  }
093  
094  public class LMBE1 {
095      public static void main(String[] args) {
096          Passenger john = new Passenger("John Gruber", "AC102");
097          john.addItem("Knives");
098          john.addItem("Teddy bear");
099          AirportTerminal at = new AirportTerminal(john);
100          at.boardAirplane();
101      }
102  }
103