Clean Code Experience

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.

Advertisement