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

[JavierCane]

Feel free to merge and commit the review suggestions afterwards in main if you prefer so 😊

[JavierCane]

👀 😬

[JavierCane]

👀 😬

[JavierCane]

It would be awesome to talk about the problem domain in the example title instead of revealing the illustrated code smell.

Why:

• Avoid giving hints on how to approach the solution
• Avoid collision with other examples for this very same code smell
• Avoid having to decide which code smell include in the example name in cases where multiple of them are illustrated
• Be able to be consisten about the example "branding' between languages. That is, not having to include the language in its folder name

Alternative suggestion: course_steps_duration_calculator

[JavierCane]

🎩