Majority of developers sometimes in their lives have tried to refactor a code that is highly coupled with one or multiple data classes. This is a way to work with it without crying like a baby.
Let’s focus in I of SOLID:
Interface segregation: many client-specific interfaces are better than one general-purpose interface
So imagine we have a class called Hotel:
@Value
public class Hotel
{
private final String hotelId;
private final List<MealPlan> mealPlans;
private final BasicStaticInfo staticInfo;
}and that class is used to sort and to filter.
@AllArgsConstructor
public class BudgetFilter{
private final long minBudget;
private final long maxBudget;
public boolean passes(List<Hotel> hotels){
return hotels.stream()
.anyMatch(h -> h.getMealPlans().stream.anyMatch(m -> m.getPrice() <= maxBudget && m.getPrice() >= minBudget));
}
}
@AllArgsConstructor
public class SortByPopularity{
public List<Hotel> sort(List<Hotel> hotels){
return hotels.stream()
.sorted((a,b) -> b.getStaticInfo().getPopularity() - a.getStaticInfo().getPopularity()).collect(Collectors.toList());
}
}Let’s also take into account tests we have created to test our services.
public class BudgetFilterTest{
@Test
public void emptyList(){
BudgetFilter budgetFilter = new BudgetFilter(0l,0l);
assertThat(budgetFilter.passes(emptyList()), is(true));
}
@Test
public void oneMealPlanMinBudget(){
BudgetFilter budgetFilter = new BudgetFilter(30l, 100l);
assertThat(budgetFitler.passes(singletonList(new Hotel(null, singletonList(new MealPlan(null, 30l)), null)), is(true));
}
@Test
public void oneMealPlanMaxBudget(){
BudgetFilter budgetFilter = new BudgetFilter(30l, 100l);
assertThat(budgetFitler.passes(singletonList(new Hotel(null, singletonList(new MealPlan(null, 100l)), null)), is(true));
}
@Test
public void oneMealPlanInsideTheBudget(){
BudgetFilter budgetFilter = new BudgetFilter(30l, 100l);
assertThat(budgetFitler.passes(singletonList(new Hotel(null, singletonList(new MealPlan(null, 50l)), null)), is(true));
}
@Test
public void oneMealPlanOutsideTheBudget(){
BudgetFilter budgetFilter = new BudgetFilter(30l, 100l);
assertThat(budgetFitler.passes(singletonList(new Hotel(null, singletonList(new MealPlan(null, 110l)), null)), is(false));
}
@Test
public void twoMealPlansOneInsideOneOutsideTheBudget(){
BudgetFilter budgetFilter = new BudgetFilter(30l, 100l);
assertThat(budgetFitler.passes(singletonList(new Hotel(null, Arrays.asList(new MealPlan(null, 110l), new MealPlan(null, 50l)), null)), is(true));
}
}
public class SortByPopularityTest{
@Test
public void emptyList(){
SortByPopularity sortByPopularity = new SortByPopularity();
assertThat(sortByPopularity.sort(emptyList()), is(emptyList()));
}
@Test
public void twoHotelDisordered(){
SortByPopularity sortByPopularity = new SortByPopularity();
assertThat(sortByPopularity.sort(Arrays.asList(
new Hotel(null, null, new BasicStaticInfo(50)),
new Hotel(null, null, new BasicStaticInfo(75)))),
is(Arrays.asList(
new Hotel(null, null, new BasicStaticInfo(75)),
new Hotel(null, null, new BasicStaticInfo(50))))));
}
}In this situation imagine that we have a performance problem because our data store size limitations (we are storing our search results in our data store to avoid calling for every operation to our providers) and we have increased the number of searches our users do.
So our conclusion is to stop using the whole Hotel object to store the information in our data store but:
- We are storing mealplans because of BudgetFilter
- We are storing staticInfo because of our SortByPopularity service.
As we haven’t followed Interface Segregation principle nor Law of Demeter, in some way the module in charge of storing data in our data store is coupled with methods and memory structures used by our services and our services between them.
What to do?
- We could try to save the world at once removing the Hotel object and replacing it with another little class. This approach is hard, because usually everything will be broken and probably you will have to expend days to solve it and later manage the conflicts because of other developers work.
- We can try with step by step plan:
- We could apply Law of Demeter so we could add to hotel methods to avoid giving access to deeper classes.
- Replacing in
BudgetFilterandSortByPopularitycode by these new methods. - Extracting two interfaces in
Hotelone calledFilterableHoteland one calledSortableHotel - Use those interfaces in
BudgetFilterandSortByPopularityservice
1. Law of Demeter
@Value
public class Hotel
{
private final String hotelId;
private final List<MealPlan> mealPlans;
private final BasicStaticInfo staticInfo;
+ public boolean isPriceInBudget(double minPrice, double maxPrice){
+ return mealPlans.stream.anyMatch(m -> m.getPrice() <= maxBudget && m.getPrice() >= minBudget);
+ }
+ public int getPopularity() {
+ return staticInfo.getPopularity();
+ }
}2. Using new methods in services
@AllArgsConstructor
public class BudgetFilter
private final long minBudget;
private final long maxBudget;
public boolean passes(List<Hotel> hotels){
return hotels.stream()
- .anyMatch(h -> h.getMealPlans().stream.anyMatch(m -> m.getPrice() <= maxBudget && m.getPrice() >= minBudget));
+ .anyMatch(h -> h.isPriceInBudget(minBudget, maxBudget)));
}
}
@AllArgsConstructor
public class SortByPopularity
public List<Hotel> sort(List<Hotel> hotels){
return hotels.stream()
- .sorted((a,b) -> b.getStaticInfo().getPopularity() - a.getStaticInfo().getPopularity()).collect(Collectors.toList());
+ .sorted((a,b) -> b.getPopularity() - a.getPopularity()).collect(Collectors.toList());
}
}3. Creating interfaces
public interface FilterableHotel
{
boolean isPriceInBudget(double minBudget, double maxBudget);
}
public interface SortableHotel
{
int getPopularity();
}@Value
-public class Hotel
+public class Hotel implements FilterableHotel, SortableHotel
{
private final String hotelId;
private final List<MealPlan> mealPlans;
private final BasicStaticInfo staticInfo;
public boolean isPriceInBudget(double minBudget, double maxBudget){
return mealPlans.stream.anyMatch(m -> m.getPrice() <= maxBudget && m.getPrice() >= minBudget);
}
public int getPopularity() {
return staticInfo.getPopularity();
}
}4. Using the new interfaces in our services
@AllArgsConstructor
public class BudgetFilter
private final long minBudget;
private final long maxBudget;
- public boolean passes(List<Hotel> hotels){
+ public boolean passes(List<? extends FilterableHotel> hotels){
return hotels.stream()
.anyMatch(h -> h.isPriceInBudget(minBudget, maxBudget)));
}
}
@AllArgsConstructor
public class SortByPopularity
- public List<Hotel> sort(List<Hotel> hotels){
+ public List<SortableHotel> sort(List<? extends SortableHotel> hotels){
return hotels.stream()
.sorted((a,b) -> b.getPopularity() - a.getPopularity()).collect(Collectors.toList());
}
}In short, we are saying that:
- We can move these responsibilities step by step, since these are small, safe refactors (
BudgetFilterTestandSortByPopularityTestwill remain green). - Once you moved these responsibilities to the interfaces, we’re free to alter the data structure itself, optimizing it.
Once this plan is executed our services have been decoupled from the Hotel class so we can start thinking how to refactor Hotel for improvements or even removing the class.
