logo
Articleoodesign

Single responsibility principle and testing

Views: 5838 Created: 2019-06-25 Read time: 10 minutes
Post Preview
Tags:

1)     Preface

Imagine that you are a proud owner of a freshly registered startup. You have that great sense of pride in you. The business plan is already there, but there is a bit of anxiety in you regarding the future. How will my startup tackle with the fierce competition? How am I supposed to choose the right staff? Will the idea be actually useful?

You get into an elevator while still fighting with all these thoughts. The door closes, but just before it shut, someone holds it and gets in. A neatly dressed businessmen stands just beside you. He is finishing his discussion on his phone. You can infer easily that he is closing an investment deal. Once he hangs off, I will approach him, you think to yourself. Why wait for another chance if there it is, standing next to me.

Now imagine that your business plan does not communicate a clear vision. Maybe it is not that bad, but it takes a while to understand. There are too many variables, and it seems that you want to cover too much ground at once. You exchange a few words with the businessman and proceed to introduce your company and its vision. After a few sentences, he is lost in the sea of details, and it is obvious he just wants to get out of the elevator as fast as possible. This is precisely what happens. You have lost your first chance, and no one knows how long you will have to wait for another shot at getting some investments.

The idea is the same when it comes to designing software. If the responsibility of a module or a single class cannot be easily described, then you can be sure that it is trying to do way too much. You should be aware at this point that you are only getting yourself and your team into trouble.

 

2)     What questions are relevant for single responsibility principle?

The most obvious thought that comes into mind when trying to imagine SRP in practice is "do one thing and one thing only". In general, that fits in the definition naturally. How about we approach the principle in another way. Let us ask the following question: "Is there more than one reason for the unit to change". Ahh, that changes the perspective while still keeping that "one thing" spirit of the first statement.

SRP is a straightforward principle, but it might be one of the hardest to get just right. Apart from "the one reason to change" question I always like to couple it with another one: "Am I able to describe the nature of the unit in one sentence". This tandem should make our decisions a bit easier. The borders are still a bit vague, but we have something tangible to grab on to.

Let us try to go through an example. We will be working on a feature of a game that is responsible for managing the levelling up of a hero:

 

SUT Code


public class HeroManager{

	public void levelUp(Hero hero){
		hero.setLevel(hero.getLevel() + 1);

		if(hero.getLevel() % 10 == 0){
			hero.addSpell(gameEngine.generateSpecialSpell());
		}

		if(hero.getLevel() % 5 == 0){
			hero.addMoney(gameEngine.generateBonusMoney());
		}

		if(hero.getActiveBuff() == null){
			hero.setActiveBuff(gameEngine.generateRandomBuff());
		}
	}

}

The unit seems to be quite straightforward:

      (L5):  Each and every hero gets his level incremented by exactly one.

      (L7):  Every ten levels add a special bonus spell to our hero.

      (L11):  Every five levels give our hero some extra money he can spend at the local tavern just to let out some steam after all those battles he won.

      (L15):  Add a random buff to our hero that temporarily increases one of his main stats.

Let's ask ourselves the SRP questions:

Is there only one reason to change?

Answer A: Yes, anything regarding the level processing would go in this unit. So only this feature affects it.

Answer B: Not quite, sometimes the level might be increased by more than one. We may also need to add more and more bonuses to the user depending on the level. This might bloat the unit considerably.

Answer C: The active buff is not really dependent on the exact level. The reason for the change here would be different when adding a spell on the money.

 

Am I able to describe the nature of the unit in one sentence?

Answer A: Yes, we are making our hero's level go up.

Answer B: No, there is too much going on apart from the increase itself. The name of the class is too broad altogether.

Answer C: Yes, we are increasing our hero's level and adding spells if adequate and throwing in some money every couple of levels and buffing his primary stats, whatever that means.  

 

Ok, so the point of view is a bit distributed, to say the least on any of these questions. Let us go after the voices which say the responsibility is not sharp enough and see where can we end up:

 

Refactored Code


public class HeroLevelManager{

	private static final Integer INCREMENT = 1;
	private static final Integer SPELL_BREAKPOINT = 10;
	private static final Integer MONEY_BREAKPOINT = 5;

	public void levelUp(Hero hero){
		hero.setLevel(hero.getLevel() + INCREMENT);

		resolveLevelBasedBonuses(hero);
		generateBuff(Hero hero);
	}

	private void resolveLevelBasedBonuses(Hero hero){
		if(hero.getLevel() % SPELL_BREAKPOINT == 0){
			hero.addSpell(gameEngine.generateSpecialSpell());
		}

		if(hero.getLevel() % MONEY_BREAKPOINT == 0){
			hero.addMoney(gameEngine.generateBonusMoney());
		}
	}

	private void generateBuff(Hero hero){
		if(hero.getActiveBuff() == null){
			hero.setActiveBuff(gameEngine.generateRandomBuff());
		}
	}

}

What we did here is basically an internal refactoring. Definitely, the public method became much cleaner and readable. We have also made constants out of some of the magic numbers to enhance the clarity even more. That is great, but the additional responsibility is still part of the main public method. It has just been moved to private members. In theory, we did nothing towards SRP.

Let's give it one more try:

 

Refactored Code


public class HeroLevelManager{

	private static final Integer INCREMENT = 1;

	private LevelEnhancementManager levelEnhancementManager;

	public void levelUp(Hero hero){
		hero.setLevel(hero.getLevel() + INCREMENT);

		levelEnhancementManager.enhance(Hero hero);
	}

}

public class LevelEnhancementManager{

	private LevelBreakpointBonusHandler levelBreakpointBonusHandler;
	private LevelBuffHandler levelBuffHandler;

	public void enhance(Hero hero){
		levelBreakpointBonusHandler.handleBonuses(hero);
		levelBuffHandler.generateBuffs(hero);
	}
}

public class LevelBreakpointBonusHandler{

	private static final Integer SPELL_BREAKPOINT = 10;
	private static final Integer MONEY_BREAKPOINT = 5;

	public void handleBonuses(Hero hero){
		if(hero.getLevel() % SPELL_BREAKPOINT == 0){
			hero.addSpell(gameEngine.generateSpecialSpell());
		}

		if(hero.getLevel() % MONEY_BREAKPOINT == 0){
			hero.addMoney(gameEngine.generateBonusMoney());
		}
	}
}

public class LevelBuffHandler{

	public void generateBuffs(Hero hero){
		if(hero.getActiveBuff() == null){
			hero.setActiveBuff(gameEngine.generateRandomBuff());
		}
	}

}

Now we may have satisfied a bit our SRP fanatics. We have done the following:

      (L27):  All the bonus processing that is dependent on the level has been moved to a specialised class LevelBreakpointBonusHandler. Here we can safely add, remove any breakpoint bonuses that we need.

      (L43):  All the decisions regarding additional buff and types have been moved to a specialised class LevelBuffHandler.

      (L16):  Any kind of enhancement to the hero during the level up has been wrapped up in this LevelEnhancementManager class. It is not responsible for any logic. It just references the aforementioned specialised handlers.

      (L8):  Our original HeroLevelManager class has now really only one reason to change. That is the increment. There is nothing more because any change to the enhancements is encapsulated within the LevelEnhancementManager. Even if a requirement is presented to remove all the enhancement management, then we can inject an empty implementation. The HeroLevelManager would still remain unchanged.

 

Tip
Tip

 

Note
Note

 

3)     3) Single responsibility principle: the shortcut

The human brain likes to simplify things. That is a fact. Especially management brain. These types see only numbers and dates with people falling under the numbers tag. Digressions aside, when we consider the broad idea of SRP, we naturally want to put some kind of physical borders around it. Here are the most common ones that I have found teams tend to use:

      When a class has more than 3 fields, it should be split (apart from DTO/Entity classes)

      When a method has more than two conditional levels (loops included) it should be broken down.

      When a class has more than 3 public methods, it should be split.

      When a method has more than 40 lines, we should break it down.

We may go on, of course, but we can clearly see where this is leading. Imagine that every time you go out on a date, you carry a set of rules that your dad gave to pick that perfect moment to go for the first kiss:

      You had to talk to her for at least two hours

      She is laughing at your stupid jokes

      She is playing with her hair

      She cannot get her eyes off you. Even when her phone is buzzing every 30 seconds with notifications. (if you find one like that let me know, she should be examined for the good of mankind).

      You feel comfortable and natural in her presence

Now you could go for the kiss after all of these are met, some of these are met or only one. It depends on the situation. That is why you are an engineer, and you have to figure it out right there on the spot. Forget about strict rules in the SRP context. Think of SRP in terms of women. You never know for sure.

 

Note
Note

 

4)     Unit tests as the single responsibility principle barometer

The ideal option, in my opinion, when it comes to deciding whether to use SRP or not is to use unit testing. How can unit tests act as a watchdog of SRP? There is a very unique symbiosis between software craftsmanship and unit testing (automated testing in general also). When it comes to software craftsmanship, we are making sure that technically everything is written in a way that guarantees the highest quality of code. When it comes to automated testing, we are making sure that we consistently get the highest quality in terms of functionality. Combine them together, and your software is literally unstoppable.

Now how does SRP let's our unit tests thrive, and unit tests make sure our code meets the SRP? Let us look at the following example of a restaurant management application where the customer tries to pay for his order:

 

SUT Code


public static Bill processBill(Customer customer, Waiter waiter, Order order){
	Bill bill = waiter.prepareBillForOrder(order);
	Iterator<CreditCard> creditCards = customer.prepareToPay();

	while(!bill.paid()){
		if(creditCards.hasNext()){
			CreditCard creditCard = creditCards.next();

			TransactionResult transactionResult = TransactionResult.WRONG_PIN;

			while(TransactionResult.WRONG_PIN.eqauls(transactionResult)){
				transactionResult = waiter.performTransaction(bill, creditCard);

				if(transactionResult == TransactionResult.WRONG_PIN){
					waiter.informPinNotAccepted(customer, bill);
				}
			}

			if(transactionSuccessful){
				bill.setPaid(true);
				break;
			}else{
				waiter.informTransactionNotAccepted(customer, bill);
			}
		}else{
			throw new RuntimeException("Customer cannot pay");
		}
	}

	return bill;
}

Now let's try to think how could we name unit tests that would be verifying this method. We want it to comply with the BDD principle:

 

Test Code


@Test
void shouldWaiterProcessCustomerOrder_givenFirstTransactionAccepted()

@Test
void shouldWaiterRepeatCardTransaction_whenProcessingCustomerOrder_givenWrongPinHasBeenProvidedForCard()

@Test
void shouldWaiterInformTransactionNoAccepted_whenProcessingCustomerBill_givenFirstCardTransactionUnsuccessful()

@Test
void shouldThrowException_whenProcessingCustomerBillByWaiter_givenNoneOfTheCustomerCardTransactionsWasSuccessfull();

These do not look good, do not sound right, and possibly they even do not smell that good either. Our method definitely tries to do too much. We simply cannot come up with meaningful unit test names that are not a mile long. That is the indicator. We lost contact with SRP here. Let's try to refactor a bit the method and think about the test names once again:

 

Refactored Code


public Bill processOrder(Customer customer, Order order){
	Bill bill = waiter.prepareBillForOrder(order);
	Iterator<CreditCard> creditCards = customer.prepareToPay();

	while(!bill.paid()){
		if(creditCards.hasNext()){
			bill.setPaid(payForBill(creditCards.next(), customer));
		}else{
			throw new RuntimeException("Customer cannot pay");
		}
	}

	return bill;
}

public boolean payForBill(CreditCard creditCard, Bill bill){
	TransactionResult transactionResult = TransactionResult.WRONG_PIN;

	while(TransactionResult.WRONG_PIN.eqauls(transactionResult)){
		transactionResult = performTransaction(bill, creditCard);

		if(transactionResult == TransactionResult.WRONG_PIN){
			informPinNotAccepted(customer, bill);
		}
	}

	if(TransactionResult.SUCCESSFUL.equals(transactionResult)){
		bill.setPaid(true);
		return true;
	}else{
		informTransactionNotAccepted(customer, bill);
	}

	return false;
}

We have extracted the card processing details and moved the methods to the Waiter class. Thanks to that we will not need to use that word in our tests as the context will be obvious. Thanks to refactoring, we have two specialised methods instead of one. Now we can write unit tests for these separately. Let's he how it went:

 

Test Code


@Test
void shouldRepeatCardTransaction_givenWrongPinHasBeenProvided()

@Test
void shouldInformTransactionNotAccepted()

@Test
void shouldMarkTheBillAsPaid_givenCardTransactionSuccessful()

 

Test Code


@Test
void shouldProcessOrder_givenFirstTransactionAccepted()

@Test
void shouldProcessOrder_givenSubsequentTransactionAccepted()

@Test
void shouldThrowException_whenProcessingCustomerOrder_givenNoCustomerCardTransactionsWasSuccessfull();

 

This looks a lot better. Both the refactored method and the unit test naming. I think we are quite there on the right path. Some of the methods might seem still a bit long though. Do you think we should refactor even further?

 

Tip
Tip

 

Reference
Reference
oodesign
Test method naming

 

5)     Conclusion

 

Summary

 

SRP is the king of software design principles, but yet it's ethereal nature makes it a hard task to implement. Thankfully when we ask the right questions and validate the state of our unit testing suite, we can adjust and introduce just the right changes. There are no hard rules, though, and every case is most likely to be a bit different than all the rest.

Need more insight?
Repository
Repository
Glossary
Glossary
Tags:
Reference
You may also like:
tdd-refactoring-phase
TDD refactoring phase
test-method-naming
Test method naming
Comments
Be the first to comment.
Leave a comment