2
\$\begingroup\$

I have a scenario to design a class structure to creates different types of derived class objects and set some of their common base class properties. Here is the final pattern I came up with to solve the problem. Can someone please take a look and advise if this a good design or can it be improved?

The domain model's ContractEmployee and FullTimeEmployee inherits from the base class Employee. The two properties of the base class DailyWorkHours and WeeklyHolidays will always have the same values for every Employee sub type.

The employee objects needs to be created using their respective Builder classes such as a ContractEmployeeBuilder or FullTimeEmployeeBuilder. These builder classes are exposed to the client using interfaces.

The Builder classes are responsible for creating the respective employee objects, set their properties as well as their base class properties. However I do not want every concrete builder class to to have the repeated code of creating the objects and setting the base class properties. So I have created a base class named EmployeeBuilderBase to achieve this.

The abstract EmployeeBuilderBase class can create the object, set common base class properties and calls an abstract method named SetProperties. The builder classes can override this method to perform any additional operations they want to do on the object.

/***Code Below*****/
using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace EmployeeApp.Tests
{
    [TestClass]
    public class EmployeeBuilderTests
    {
        [TestMethod]
        public void CreateContractEmployee_ReturnsAContractEmployeeObject()
        {
            IContractEmployeeBuilder contractemployeeBuilder = new ContractEmployeeBuilder();
            ContractEmployee emp = contractemployeeBuilder.CreateContractEmployee(12, "ABC ltd.");
            Assert.AreEqual(emp.ContractDurationInMonths, 12);
            Assert.AreEqual(emp.DailyWorkHours, 8);
        }

        [TestMethod]
        public void CreateFullTimeEmployee_ReturnsAFullTimeEmployeeObject()
        {
            IFullTimeEmployeeBuilder fulltimeEmployeeBuilder = new FullTimeEmployeeBuilder();
            FullTimeEmployee emp = fulltimeEmployeeBuilder.CreateFullTimeEmployee("John Smith", 2500);
            Assert.AreEqual(emp.MonthlySalary, 2500);
            Assert.AreEqual(emp.DailyWorkHours, 8);
        }
    }

    ///*************************Domain Models*********************************************    
    public class Employee
    {
        public int DailyWorkHours { get; set; }
        public int WeeklyHolidays { get; set; }
    }

    public class ContractEmployee : Employee
    {
        public int ContractDurationInMonths { get; set; }
        public string AgencyName { get; set; }
    }

    public class FullTimeEmployee : Employee
    {
        public int MonthlySalary { get; set; }
        public string EmployeeName { get; set; }
    }

    ///*************************Interfaces and Abstract base class for the Builder Classes*********************************************    
    public interface IContractEmployeeBuilder
    {
        ContractEmployee CreateContractEmployee(int contractDurationInMonths, string agencyName);
    }

    public interface IFullTimeEmployeeBuilder
    {
        FullTimeEmployee CreateFullTimeEmployee(string employeeName, int monthlySalary);
    }

    public abstract class EmployeeBuilderBase<T> where T : Employee, new()
    {
        public T BuildEmployee(object options)
        {
            T obj = new T();
            obj.DailyWorkHours = 8;
            obj.WeeklyHolidays = 2;
            SetProperties(obj, options);
            return obj;
        }

        public abstract void SetProperties(T obj, object options);
    }

    //********Concrete Builder Classes***********************************
    public class ContractEmployeeBuilder : EmployeeBuilderBase<ContractEmployee>, IContractEmployeeBuilder
    {
        public ContractEmployee CreateContractEmployee(int contractDurationInMonths, string agencyName)
        {
            ContractEmployee contractor =
                BuildEmployee(new {ContractDurationInMonths = contractDurationInMonths, AgencyName = agencyName});
            return contractor;
        }

        public override void SetProperties(ContractEmployee obj, object options)
        {
            obj.ContractDurationInMonths = options.GetPropertyValue<int>("ContractDurationInMonths");
            obj.AgencyName = options.GetPropertyValue<string>("AgencyName");
        }
    }

    public class FullTimeEmployeeBuilder : EmployeeBuilderBase<FullTimeEmployee>, IFullTimeEmployeeBuilder
    {
        public FullTimeEmployee CreateFullTimeEmployee(string employeeName, int monthlySalary)
        {
            FullTimeEmployee emp = BuildEmployee(new {EmployeeName = employeeName, MonthlySalary = monthlySalary});
            return emp;
        }

        public override void SetProperties(FullTimeEmployee obj, object options)
        {
            obj.MonthlySalary = options.GetPropertyValue<int>("MonthlySalary");
            obj.EmployeeName = options.GetPropertyValue<string>("EmployeeName");
        }
    }

    //**** Object Extension method******
    public static class ObjectExtensions
    {
        public static T GetPropertyValue<T>(this object obj, string propName)
        {
            object value = obj.GetType().GetProperty(propName).GetValue(obj, null);
            return (T) value;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Hi, welcome to CR. Your code is broken: 1) It doesn't compile (GetValue has no overload that accepts only one parameter). 2) Your example will fail at runtime in SetProperties because you're using the wrong property names - this coincidentally points out one of the biggest flaws in your approach. In any case we generally don't review broken code. \$\endgroup\$ – ChrisWue Jun 30 '15 at 0:50
  • 1
    \$\begingroup\$ @ChrisWue - .Net 4.5 introduced a GetValue overload with only a single parameter. \$\endgroup\$ – RobH Jun 30 '15 at 10:46
  • \$\begingroup\$ Thank you. I have corrected the code. Please take a look now. \$\endgroup\$ – suneeth Jun 30 '15 at 11:10
  • \$\begingroup\$ @RobH: Yea, my fault. For some reason my test project was targeting 3.5 \$\endgroup\$ – ChrisWue Jul 1 '15 at 1:28
  • \$\begingroup\$ @ChrisWue I have corrected the code. Please take a look if you get a chance. \$\endgroup\$ – suneeth Jul 2 '15 at 15:53
3
\$\begingroup\$

ContractDuration has no point of reference. Example: "6 months" from when? My suggestion would be to include a DateTime for the contract effective date, and then perhaps change the ContractDuration to be a TimeSpan.

\$\endgroup\$
  • \$\begingroup\$ thank you. forget about datatypes. I just wrote this code to illustrate the scenareo . what I am interested in is to see if anyone can suggest me a better design pattern for this problem. \$\endgroup\$ – suneeth Jun 30 '15 at 11:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.