Thoughts about Feature Envy and path to follow when refactoring legacy code

1. When writing tests for legacy code we go from shortest to deepest branches.
2. When refactoring legacy code (handmade modifications) we go from deepest to shortest branches (just the opposite)

Feauture envy: some class (TripService) wants to do some action that should be implemented by another class which we
are using to extract the data and perform some action. Data and action should be done by the same class (User)

diff --git a/TDD/src/main/java/org/craftedsw/tripservicekata/trip/TripService.java b/TDD/src/main/java/org/craftedsw/tripservicekata/trip/TripService.java
index 3eec3a8..570b6a0 100644 (file)
--- a/TDD/src/main/java/org/craftedsw/tripservicekata/trip/TripService.java
+++ b/TDD/src/main/java/org/craftedsw/tripservicekata/trip/TripService.java
@@ -17,12 +17,22 @@ public class TripService {
                User loggedUser = getLoggedInUser();\r
                boolean isFriend = false;\r
                if (loggedUser != null) {\r
+                       // Feature envy. TripService envies User class. This should be done by User class.\r
                        for (User friend : user.getFriends()) {\r
                                if (friend.equals(loggedUser)) {\r
                                        isFriend = true;\r
                                        break;\r
                                }\r
                        }\r
+                       // The deepest branch. For refactoring legacy code we must begin from the\r
+                       // deepest branch. This is just the opposite for creating the unit test\r
+                       // for our legacy code.\r
+                       // 1. Write the unit test from the shortest to deepest branch. Modifications\r
+                       // in legacy code must be done JUST with the automatic tools provided by the IDE.\r
+                       // 2. Once the legacy code is under test start refactoring from deepest\r
+                       // to shortest branch. Modifications in the legacy code may be hand made.\r
+                       // In this case we can not do anything with this code (this is the deepest branch)\r
+                       // so we will have to start with the for loop (which is the second deepest branch)\r
                        if (isFriend) {\r
                                tripList = tripsBy(user);\r
                        }\r