A couple days ago, John MacIntyre [@JohnMacIntyre] posted an article about refactoring a small wage calculator function. This is John’s second post in a series dealing with his reactions to Robert C. Martin‘s book Clean Code. First of all, thanks and kudos to John for sharing his experiences!

I thought I’d give it a whirl as well, and decided to actually blog about it too (this is my first meaningful post dear reader; please be gentle! 😉 ).

I focused on the DRY issue that John noted in his analysis: that the base pay calculations were duplicated in *WageCalculatorForEmployee::CalculateWithoutOvertime* and *WageCalculatorForContractor::Calculate*. To resolve this, I decided to split up the methods to calculate regular and overtime hours as well as the methods to calculate the corresponding pays.

I retained the 3 classes that John established, but modified them as follows:

public abstract class WageCalculatorBase { public const Decimal MAXIMUM_BASE_WEEKLY_HOURS = 40; public const Decimal MAXIMUM_TOTAL_WEEKLY_HOURS = 80; public Decimal HoursWorked { get; protected set; } public Decimal HourlyRate { get; protected set; } public WageCalculatorBase(Decimal hours, Decimal hourlyRate) { if (hours MAXIMUM_TOTAL_WEEKLY_HOURS) throw new ArgumentOutOfRangeException( String.Format("Hours must be between 0 and {0}.", MAXIMUM_TOTAL_WEEKLY_HOURS)); HoursWorked = hours; HourlyRate = hourlyRate; } public Decimal Calculate() { return CalculateBasePay() + CalculateOverTimePay(); } protected Decimal CalculateBasePay() { return HourlyRate * BaseHours; } protected abstract Decimal CalculateOverTimePay(); protected Decimal BaseHours { get { return HoursWorked <= MAXIMUM_BASE_WEEKLY_HOURS ? HoursWorked - MAXIMUM_BASE_WEEKLY_HOURS : 0; } } protected Decimal OvertimeHours { get { return HoursWorked > MAXIMUM_BASE_WEEKLY_HOURS ? HoursWorked - MAXIMUM_BASE_WEEKLY_HOURS : 0; } } }

public class WageCalculatorForEmployee : WageCalculatorBase { protected const Decimal OVERTIME_MULTIPLIER = 1.5m; public WageCalculatorForEmployee(Decimal hours, Decimal hourlyRate) :base(hours, hourlyRate) { } protected override Decimal CalculateOverTimePay() { return OvertimeHours * OVERTIME_MULTIPLIER * HourlyRate; } }

public class WageCalculatorForContractor : WageCalculatorBase { public WageCalculatorForContractor(Decimal hours, Decimal hourlyRate) :base(hours, hourlyRate) { } protected override Decimal CalculateOverTimePay() { return OvertimeHours * HourlyRate; } }

Shortly after John’s post (and after I had sent a copy of my refactoring to John), I saw the response provided by Cory Fowler [@SyntaxC4], as well as another by Jon Skeet [@jonskeet]. Both certainly are interesting and provided me with some new insights. They also added to the original requirements by considering a person who earns a different rate for various amounts of overtime worked. I decided to try to implement something like this as well, and was able to do so without having to alter my originally refactored design. Given this, I figure the Template Method approach was a decent fit to the problem.

public class WageCalculatorForMultiRateContractor : WageCalculatorBase { protected const Decimal BASE_OVERTIME_HOURS_SPAN = 10.0m; protected const Decimal EXTENDED_OVERTIME_HOURS_SPAN = 20.0m; protected const Decimal MAXIMAL_OVERTIME_HOURS_SPAN = 10.0m; protected const Decimal BASE_OVERTIME_MULTIPLIER = 1.5m; protected const Decimal EXTENDED_OVERTIME_MULTIPLIER = 2.0m; protected const Decimal MAXIMAL_OVERTIME_MULTIPLIER = 2.5m; public Decimal BaseOvertimeHours { get { return Math.Min(OvertimeHours, BASE_OVERTIME_HOURS_SPAN); } } public Decimal ExtendedOvertimeHours { get { var moreThanBaseOvertimeHours = Math.Max(OvertimeHours - BASE_OVERTIME_HOURS_SPAN, 0); return Math.Min(moreThanBaseOvertimeHours, EXTENDED_OVERTIME_HOURS_SPAN); } } public Decimal MaximalOvertimeHours { get { var moreThanExtendedOvertimeHours = Math.Max(OvertimeHours - BASE_OVERTIME_HOURS_SPAN - EXTENDED_OVERTIME_HOURS_SPAN, 0); return Math.Min(moreThanExtendedOvertimeHours, MAXIMAL_OVERTIME_HOURS_SPAN); } } public WageCalculatorForMultiRateContractor(Decimal hours, Decimal hourlyRate) :base(hours, hourlyRate) { } protected override Decimal CalculateOverTimePay() { var baseOvertimePay = BaseOvertimeHours * BASE_OVERTIME_MULTIPLIER * HourlyRate; var extendedOvertimePay = ExtendedOvertimeHours * EXTENDED_OVERTIME_MULTIPLIER * HourlyRate; var maximalOvertimePay = MaximalOvertimeHours * MAXIMAL_OVERTIME_MULTIPLIER * HourlyRate; return baseOvertimePay + extendedOvertimePay + maximalOvertimePay; } }

Copyright © Cliff Mees 2010; all rights reserved.

Welcome to the Blogosphere Cliff. You should’ve started this a long time ago. You’ve got a lot to share and I’m looking forward to your future posts.

John

Way to go Cliff, but you are such a geek!

Pingback: My week (09/18/2010) « While I Compile