Reply: What is too simple and small to refactor

This blog post is in reply to John MacIntyre’s [@JohnMacIntyre] post What is too simple and small  to refactor? I’ll wait here while you shoot off to his blog to get some context.

John began his refactor due to a Boolean parameter in Jim Holmes’ method. He managed to remove the Boolean as a Parameter but still ended up maintaining the Boolean in his Code even though he refactored the class to introduce a base class.

A while ago, I was reading about [I’ll update this entry with a link when I track it down again] a movement in the programming community removing conditional statements through abstraction. This lead be to believe I could remove the Boolean from Johns implementation and get the Classes down to something a little bit simpler and introducing a little bit more reusability.

I Introduced a struct to the mix to ensure that there is a simple  type safe method of calculating overtime hours. I chose a struct over a class as it provides a smaller memory footprint.

// Developer: Cory Fowler
// Company: SyntaxC4 Development Services
// Description: Adds Type Safety and Saves Memory over Class

namespace WageCalculator.Abstractions
{
    public struct OvertimeRates
    {
        public const float Regular = 1.0f;
        public const float Time_And_A_Half = 1.5f;
        public const float Double_Time = 2.0f;
        public const float Double_Time_And_A_Half = 2.5f;
    }
}

Next I created the base class which is to provide the common functionality between the different implementations of our class.

// Developer: Cory Fowler
// Company: SyntaxC4 Development Services
// Description: Exposes Common Functionality to WageCalculators.

namespace WageCalculator.Abstractions
{
    #region Usings
    using System;
    #endregion

    public abstract class WageCalculatorBase
    {
        public float HoursWorked { get; private set; }
        public float HourlyRate { get; private set; }

        public const float MAXIMUM_HOURS_PER_WEEK = 80f;
        public const float MINIMUM_HOURS_PER_WEEK = 0f;
        
        public WageCalculatorBase(float hours, float hourlyRate)
        {
            if (hours < MINIMUM_HOURS_PER_WEEK || hours > MAXIMUM_HOURS_PER_WEEK)
                // Should Probably Create WorkWeekOutOfRangeException
                throw new ArgumentOutOfRangeException(string.Format("Hours must be between {0} and {1}",
                                                                                MINIMUM_HOURS_PER_WEEK,
                                                                                MAXIMUM_HOURS_PER_WEEK));
            HoursWorked = hours;
            HourlyRate = hourlyRate;
        }

        protected float CalculateOverTimeHours(float hours, 
                                               float overtimeThreshold, 
                                               Func<float, float> calculateOvertimePay)
        {
            return calculateOvertimePay(hours - overtimeThreshold);
        }

        public abstract float Calculate();
    }
}

The constructor has basically remained the same except I created some constants [which could be refactored to readonly variables and loaded from a configuration file] this allows for Testability of those values [we can ensure that the developer doesn’t try to set the max hours per week over 168 (24*7)].

I have added a method in to calculate the overtime hours, this was something that John had in one of his implementations however I thought the functionally should be present in all scenarios [just because it’s there doesn’t mean it’s necessarily used in *ALL* cases, normally just most.]  You can say I cheated a bit as I’ve used .NET 3.5 [not sure if John’s Solution was for .NET 2.0 or not] and included a method that takes a Parameter of type Func<T, TResult>.

I also determined that the Calculate functionally should be implemented per scenario as this is the main reason for the abstraction.

For the implementations of the types I’ve followed the same path as John, but took a slight detour. I noticed that John had implemented a Factory Class to resolve his WageCalculator type, which is a good practice to get into. The Factory will resolve the type that is necessary to WageCalculator Object instead of tying wage calculation to a Type of Employee [As the employee isn’t relevant here, it’s the reason why there is a factory in the first place.]

My First Concrete Implementation is a SalaryWageCalculator which doesn’t calculate overtime pay [a replacement for the WageCalculatorForContractor class].

// Developer: Cory Fowler
// Company: SyntaxC4 Development Services
// Description: Calculates Regular Time Rates for Employees

namespace WageCalculator.Concrete
{
    public class SalaryWageCalculator : Abstractions.WageCalculatorBase
    {
        #region ctor
        public SalaryWageCalculator(float hours, float hourlyRate) : 
                    base(hours, hourlyRate) { }
        #endregion

        public override float Calculate()
        {
            return HoursWorked * HourlyRate;
        }
        
        public static float Calculate(float hours, float hourlyRate)
        {
            return new SalaryWageCalculator(hours, hourlyRate).Calculate();
        }
    }
}

As you can see there is no Boolean value in the class, it simply knows that it should only calculate a standard wage with no overtime.

My second Concrete Implementation is a TimeAndAHalfWageCalculator which calculates overtime at the rate of one and a half times salary on any hours worked over 40 hours.

// Developer: Cory Fowler
// Company: SyntaxC4 Development Services
// Description: Adds Functionality for OverTime Wage Calculation

namespace WageCalculator.Concrete
{
	#region Usings
	using Abstractions;
	#endregion

	public class TimeAndAHalfWageCalculator : WageCalculatorBase
	{
		public const float MAXIMUM_HOURS_BEFORE_TIME_AND_A_HALF = 40;

		public TimeAndAHalfWageCalculator(float hours, float hourlyRate) : base(hours, hourlyRate){}
	
		public override float Calculate()
		{
			return  (CalculateOverTimeHours(HoursWorked, 
						MAXIMUM_HOURS_BEFORE_TIME_AND_A_HALF, 
                        x => x * OvertimeRates.Time_And_A_Half) +
					MAXIMUM_HOURS_BEFORE_TIME_AND_A_HALF) * HourlyRate;
		}

		public static float Calculate(float hours, float hourlyRate)
		{
			return new TimeAndAHalfWageCalculator(hours, hourlyRate).Calculate();
		}
	}
}

As you can see this implementation introduces a constant for the maximum hours worked before over time is to be applied [again this can be changed to readonly and read from a configuration file.]

It also introduces the first implementation of the method created in our base class. The Func<T, TResult> is provided to the method as a Lambda Expression [but could also be implemented as a delegate] the Expression takes one parameter which is the remainder of the maximum_hours_before_time_and_a_half and HoursWorked which is then multiplied by the rate allocated for time and a half [1.5f].

The reason I have chosen to add the maximum hours back in separately is because this class can now be used as a base class and this method doesn’t need to be re-implemented or changed to add functionality for calculating the wage for employers that pay a compounded overtime amount [i.e. – After 8 hours, time and a half; after 12 hours double time and a half. using:

public override float Calculate()
{
	return CalculateOverTimeHours(HoursWorked, 52, 
		x => x * OvertimeRates.Double_Time_And_A_Half) +
	base.Calculate();
}

The above code is the nth implementation as you can pretty much do any other type of calculation by either deriving from an existing class or creating a new implementation of the base class and changing the Calculate method.

Now, I know my blog is going to chew this code to pieces, so feel free to download the Visual Studio 2010 Solution files.

Related posts