A SO[L]ID refactoring exercise – Part 4

The solutions has once again been refactored, this time by applying the Open Closed Principle. Now the latest version of StudentService class looks like this:

public class StudentService : IStudentService
{
	private readonly IStudentRepository _studentRepository;
	private readonly IUniversityRepository _universityRepository;
	private readonly ILogger _logger;
	private readonly IStudentFactory _studentFactory;

	public StudentService(IStudentRepository studentRepository, IUniversityRepository universityRepository, ILogger logger, IStudentFactory studentFactory)
	{
		_studentRepository = studentRepository;
		_universityRepository = universityRepository;
		_logger = logger;
		_studentFactory = studentFactory;
	}

	public bool Add(string emailAddress, Guid universityId)
	{		
		_logger.Log(string.Format("Log: Start add student with email '{0}'", emailAddress));	

		if (_studentRepository.Exists(emailAddress))
		{
			return false;
		}			

		var university = _universityRepository.GetById(universityId);
		
		var student = _studentFactory.Create(emailAddress, universityId, university.Package);		
		
		_studentRepository.Add(student);
		
		_logger.Log(string.Format("Log: End add student with email '{0}'", emailAddress));

		return true;
	}

	public void AddBonusAllowances()
	{
		//...get students (IEnumerable<Student>) for bonus allowance
		foreach (Student s in students)
		{
			s.AddBonusAllowance();
		}
		//...
	}
	
	public IEnumerable<Student> GetStudentsByUniversity()
	{
		//...		
	}

	public IEnumerable<Student> GetStudentsByCurrentlyBorrowedEbooks()
	{
		//...		
	}
}

 

Pass #4 – Liskov Substitution Principle (LSP)

Subtypes must be substitutable for their base types.

As you probably noticed, the last version of StudentService exposes a new method, AddBonusAllowances, which gets a list of students (the criteria or source is irrelevant) and for each one adds a bonus allowance. Because both standard or premium students can get a bonus we can use the abstract Student class and through polymorphism call the appropriate AddBonusAllowance on the underlying object. In the current context this works fine and follows the LSP because I can safely substitute StandardStudent or PremiumStudent (subtypes) with Student (base type).

However, now there is a new requirement to add a new type of Package, ‘Unlimited’. Students from the universities that purchased this package don’t have a monthly allowance limit. Let’s consider implementing the UnlimitedStudent class:

public class UnlimitedStudent: Student
{
	public UnlimitedStudent(string emailAddress, Guid universityId)
		: base(emailAddress, universityId)
	{
		this.MonthlyEbookAllowance = 0;
	}
	
	public override void AddBonusAllowance()
	{
		throw new NotImplementedException();
	}
}

As there is no monthly allowance limit for this type of student, AddBonusAllowance doesn’t have an implementation. And because of base Student class forces it, this class now breaks the LSP.

We can no longer replace UnlimitedStudent with Student because this will have an unexpected result. Sure, we can make sure this never happens during run-time by filtering the students upon retrieval, or checking the type, or even leaving the AddBonusAllowance blank. But the design will still be inappropriate and misleading.

One way to fix this is to create another abstract class LimitedStudent which inherits the Student class and has the abstract AddBonusAllowance method. Then the StandardStudent and PremiumStudent can inherit this new class, while the UnlimitedStudent will inherit the modified Student class without needing to implement the AddBonusAllowance method.

Another way would be to extract AddBonusAllowance abstract method into an interface, IBonusAllowable, which only StandardStudent and PremiumStudent will implement. The first option is shown below:

public abstract class Student
{
	protected const int STANDARD_ALLOWANCE = 10;

	public string EmailAddress { get; private set; }
	public Guid UniversityId { get; private set; }
	public int MonthlyEbookAllowance { get; set; }
	public int CurrentlyBorrowedEbooks { get; private set; }

	public Student(string emailAddress, Guid universityId)
	{
		this.EmailAddress = emailAddress;
		this.UniversityId = universityId;			
	}			
}

public abstract class LimitedStudent : Student
{
	public LimitedStudent(string emailAddress, Guid universityId)
		: base(emailAddress, universityId)
	{}
	public abstract void AddBonusAllowance();
}

public class StandardStudent : LimitedStudent
{
	public StandardStudent(string emailAddress, Guid universityId) 
		: base(emailAddress, universityId)
	{
		this.MonthlyEbookAllowance = Student.STANDARD_ALLOWANCE;
	}

	public override void AddBonusAllowance()
	{
		this.MonthlyEbookAllowance += 5;
	}
}

public class PremiumStudent : LimitedStudent
{
	public PremiumStudent(string emailAddress, Guid universityId)
		: base(emailAddress, universityId)
	{
		this.MonthlyEbookAllowance = Student.STANDARD_ALLOWANCE * 2;
	}

	public override void AddBonusAllowance()
	{
		this.MonthlyEbookAllowance += 10;
	}
}

public class UnlimitedStudent: Student
{
	public UnlimitedStudent(string emailAddress, Guid universityId)
		: base(emailAddress, universityId)
	{
		this.MonthlyEbookAllowance = 0;
	}		
}

Now we can update the AddBonusAllowances method in the StudentService to increase allowances just for students of type LimitedStudent. Of course, the method which returns the students must also be updated to return the correct student abstraction (LimitedStudent).

public void AddBonusAllowances()
{
	//...get students (IEnumerable<LimitedStudent>) for bonus allowance		
	foreach(LimitedStudent s in students)
	{			
		s.IncreaseAllowance();
	}
	//...
}

The code above satisfies the LSP because now we can safely replace any type of student with the Student abstract class without getting unexpected results. Following the same logic, we can also replace either StandardUser and PremiumUser with LimitedStudent.

No Comments

Post a Comment