Example for Shotgun Surgery

[santakadev]

This is an example for illustrating Shotgun Surgery Code Smell, in which we compute the duration of different course Step types.

The logic for determining the duration of a Step is coupled to the step type:

• Duration(videoStep) = VideoDuration * 1.1
• Duration(quizStep) = QuestionCount * estimatedTimePerQuestion * 1.5

The 1.1 and 1.5 are factors that might represent:

• Video (1.1): increases the duration due to video pauses.
• Quiz (1.5): increases the duration due to final review of the quiz.

In this example, instead of making the behaviour cohesive to the data, we have intentionally break the cohesion by spreading the behaviour in many classes and unnecessary abstractions.

Showing Shotgun Surgery Smell

In our current context, our platform only support a few types of Steps, and we know that we will add new ones in the near future, so the lack of cohesion will be a change preventer.

To show Shotgun Surgery smell, we can try to add a new CodeExerciseStep type with a new duration logic.

For that, we can start adding a new test in GetStepDurationTest that validates the behaviour of the duration for the new type.

Then, if we follow the same patterns for adding the production code, the implementation becomes crazy:

• Add STEP_TYPE_CODE_EXERCISE constant in StepEnums class.
• Add STEP_DURATION_MULTIPLIER_CODE_EXERCISE constant in StepEnums class.
• Create StepDurationCalculatorCodeExercise class.
• Add the new calculator in StepDurationCalculatorFactory.
• Add the logic for the new type in DurationMultiplier class.
• Add STEP_DURATION_MULTIPLIER_CODE_EXERCISE to the STEP_TYPES array in StepEnums class.

Solving Shotgun Surgery Smell

If we take a look to the different abstractions, we will realize that all of them are coupled to the Step type. So we can perform some refactorings (Move method, Move constant, Inline class, Parallel change) for gaining cohesion and reduce accidental complexity.

The main focus of the refactorings should be:

• Get rid of DurationMultiplier
• Get rid of StepDurationCalculator
• Get rid of StepEnums