1

Example taken from : Agile software development : principles, patterns and practices

A new employee is added by the receipt of an AddEmp transaction. This transaction contains the employee's name, address, and assigned employee number. The transaction has three forms:

AddEmp <EmpID> "<name>" "<address>" H <hourly-rate>
AddEmp <EmpID> "<name>" "<address>" S <monthly-salary>
AddEmp <EmpID> "<name>" "<address>" C <monthly-salary> <commission-rate>

proposed employee class enter image description here Add employee transaction enter image description here

My question would it be "better" if instead of subclassing AddEmployeeTransaction we have a factory for salary classification and pay schedule?

If better is too vague a term, what are disadvantages of my design over the one proposed by Uncle Bob?

Say inside EmployeeTransaction we have SalaryClassificationFactory and we call SalaryClassificationFactory.get(SalaryDetails d)

where,

enum SalaryId{H,S,C};


struct hourly
{
    int hourlyRate;
};

struct monthly
{
    int monthlySalary;
};

struct commissioned
{
    int salary;
    int commisssionRate;
};
struct SalaryDetails
{
    SalaryId kind;
    union SalaryUnion
    {
        hourly h;
        monthly s;
       commissioned c;
    }
};
q126y
  • 1,733

2 Answers2

2

Typical factory patterns defer creation of some object(s) to some factory, allowing the factory to choose the final type or object, but they do this from a common, shared (usually simple, often abstract) interface that requests an object. For example, (from http://www.oodesign.com/factory-pattern.html)

Product ProductFactory.createProduct(ProductId id)

Here the factory can create the appropriate subclass of Product by consulting the ProductId, and presumably a product catalog. All the different types that the factory can create are requested using the same method.

It seems to me that the parameters needed to accomplish each of the three salary classification transactions are substantially different from each other. Thus, a traditional factory method pattern probably would not apply across all three.

You could hide the existing subclass-based implementation behind a some other class, but you would still need three methods, each with different parameters, and as a result, that hiding class would not, in my opinion look like a factory.

Unless you could unite the various salary classification descriptions under a common type, say as in:

AddEmployeeTransaction 
    AddEmployeeFactory.createAddEmployee ( EmployeeCompensationClassification ecc, 
                                           EmployeeInfo who )

Now this method looks like a factory pattern, but we haven't accomplished anything because all we have done is defer the same kind of problem to creating the EmployeeCompensationClassification.

If you could reduce all salary compensation classifications to that of a simple code (e.g. a classification name, or, a classification id number), I think a factory pattern would have merit.

Erik Eidt
  • 34,819
1

One of the design decisions Uncle Bob made when creating his design is that Payment Classification should be Open for Extension. Which means that adding new payment classifications shouldn't require changing nor recompiling existing code. Your design breaks that design decision, because to add new payment classification, you have to add new value to SalaryId enum, new structure and add it to SalaryUnion, forcing recompile of all code that depends on SalaryId and SalaryDetails.

But that is Uncle Bob's decision. If you yourself decide that you don't want to accept complexity created by making the design Open for Extension, you can go with your design.

Euphoric
  • 38,149