rspec/rules/S6541/java/rule.adoc

Methods should not perform too many tasks (Brain Method).

// If you want to factorize the description uncomment the following line and create the file.
//include::../description.adoc[]

== Why is this an issue?

This issue is raised when Sonar considers that a method is a 'Brain Method'.  +
A Brain Method is a method that tends to centralize its owner's class logic and generally performs too many operations.
This can include checking too many conditions, using lots of variables, and ultimately making it difficult to understand, maintain and reuse. +
It is characterized by high LOC number, high cyclomatic and cognitive complexity, and a large number of variables being used. 

=== What is the potential impact?

Brain Methods are often hard to cover with tests, because of their deep nesting, and they are error-prone, because of the many local variables they usually introduce.
Such methods will be very hard to read and understand for anyone outside who created them, and therefore hard to maintain and fix if bugs get spotted. +
They also enable code duplication since the method itself can hardly be reused anywhere else.

== How to fix it

The common approach is to identify fragments of the method's code that deal with a specific responsibility and extract them to a new method.
This will make each method more readable, easy to understand and maintain, easier to test, and more prone to be reused. + 
In this paper, the authors describe a systematic procedure to refactor this type of code smell:  https://dl.acm.org/doi/10.1145/3191314["Assessing the Refactoring of Brain Methods"].

=== Code examples

==== Noncompliant code example

[source,java]
----
void farmDailyRoutine() {
    Crops southEastCrops = getCrops(1, -1);
    Crops eastCrops = getCrops(1, 0);
    WaterContainer waterContainer = new WaterContainer();
    List<Bottle> bottles = new ArrayList<>();
    for(int i = 0; i < 10; i++) {
        var bottle = new Bottle();
        bottle.addWater(10L);
        bottle.putCap();
        bottle.shake(2);
        bottles.add(bottle);
    }
    waterContainer.store(bottles);
    
    Truck t1 = new Truck(Truck.Type.TRANSPORT);
    t1.load(waterContainer);
    if(Weather.current != Weather.RAINY) {
        WaterContainer extraWaterContainer = new WaterContainer();
        List<Bottle> extraBottles = new ArrayList<>();
        if(southEastCrops.isDry()) {
            for(LandSlot ls : southEastCrops.lands()) {
                Bottle b = new Bottle();
                b.addWater(10L);
                b.putCap();
                extraBottles.add(b);
            }
        } else {
            extraBottles.add(new Bottle());
        }
        if(eastCrops.isDry()) {
            for(LandSlot ls : southEastCrops.lands()) {
                Bottle b = new Bottle();
                b.addWater(10L);
                b.putCap();
                extraBottles.add(b);
            }
        } else {
            extraBottles.add(new Bottle());
        }
        extraWaterContainer.store(extraBottles);
        t1.load(extraWaterContainer);
    } else {
        WaterContainer extraWaterContainer = WaterSource.clone(waterContainer);
        t1.load(extraWaterContainer)
    } 
}
----

==== Compliant solution

[source,java]
----
void farmDailyRoutine() { // Compliant: Simpler method, making use of extracted and distributed logic
    Crops southEastCrops = getCrops(1, -1);
    Crops eastCrops = getCrops(1, 0);
    WaterContainer waterContainer = new WaterContainer();
    List<Bottle> bottles = getWaterBottles(10, 10L, true);
    waterContainer.store(bottles);
    
    Truck t1 = new Truck(Truck.Type.TRANSPORT);
    t1.load(waterContainer);
    if(Weather.current != Weather.RAINY) {
        WaterContainer extraWaterContainer = new WaterContainer();
        fillContainerForCrops(extraWaterContainer, southEastCrops);
        fillContainerForCrops(extraWaterContainer, eastCrops);
        t1.load(extraWaterContainer);
    } else {
        WaterContainer extraWaterContainer = WaterSource.clone(waterContainer);
        t1.load(extraWaterContainer)
    }
}
  
private fillContainerForCrops(WaterContainer wc, Crops crops) { // Compliant: extracted readable and reusable method
    if(crops.isDry()) {
        wc.store(getWaterBottles(crops.lands().size(), 10L, false));
    } else {
        wc.store(Collections.singleton(new Bottle()));
    }
}
  
private List<Bottle> getWaterBottles(int qt, long liquid, boolean shake){ // Compliant: extracted readable and reusable method
    List<Bottle> bottles = new ArrayList<>();
    for(int i = 0; i < qt; i++) {
        Bottle b = new Bottle();
        b.addWater(liquid);
        b.putCap();
        if(shake) {
            b.shake();
        }
        bottles.add(b); 
    }
    return bottles;
}
----

=== How does this work?

In this case, the method ``farmDailyRoutine`` was taking care of performing many different tasks, with nested conditions and loops, it was long and had plenty of local variables.
By separating its logic into multiple single-responsibility methods, it is reusing parts of its original duplicated code and each of the new methods is now readable and easy to understand.
They are now also easier to cover with tests, and many other parts of the owner class could benefit from using these methods.

//=== Pitfalls

//=== Going the extra mile


== Resources
=== Articles & blog posts
[bibliography]
* https://link.springer.com/book/10.1007/3-540-39538-5["Object-Oriented Metrics in Practice: Using Software Metrics to Characterize, Evaluate, and Improve the Design of Object-Oriented Systems"] by M. Lanza, R. Marinescu  +
* https://dl.acm.org/doi/10.1145/3191314["Assessing the Refactoring of Brain Methods"] by S. Vidal, I. Berra, S. Zulliani, C. Marcos, J. A. Diaz Pace  +


//=== Conference presentations
//=== Standards