Which is the better approach to check object type?

Question

Approach 1:

class Employee 
{
public:
    virtual int calculateSalary() = 0;

};

class PermanentEmployee : public Employee {
    const int salaryPerMonth;
public:
    PermanentEmployee(int sal) : salaryPerMonth(sal){}

    int calculateSalary() {
        return salaryPerMonth;
    }
};

class ContractEmployee : public Employee {
    const int wagesPerHr;
    int totalHour;
public:
    ContractEmployee(int sal) : wagesPerHr(sal), totalHour(0){}

    void setWorkingDuration(int time) {
        totalHour = totalHour + time;
    }
    int calculateSalary() {
        return wagesPerHr * totalHour;
    }
};

class Manager {
    list<Employee *> ls;
public:
    void assignWorkingHour() {
        list<Employee *>::iterator it;

        for(it = ls.begin(); it != ls.end(); it++) {
            Employee *emp = *it;
            ContractEmployee* contractEmp = dynamic_cast<ContractEmployee* >(emp);
            if(contractEmp) {
                contractEmp->setWorkingDuration(5);
            }
        }
    }
};

In problem, there are 2 type of Employee: PermanentEmployee and ContractEmployee.
There is a class called Manager which contains a list of all employee working under him.
For ContractEmployee, it has to invoke function setWorkingDuration(), which is being invoked in method assignWorkingHour of class Manager.

The problem is:
Here type of Employee is being determind by dynamic_cast operator and Manager has to know about all type of derive class of Employee.

Approach 2:

Add another member in class Employee:

enum TypeOfEmployee {CONTRACT, PERMANENT};

and check TypeOfEmployee to determine the type of Employee

Please tell me which is better or is there any alternative approach?

Answer 1

The better approach is to write code that doesn't require knowledge about the exact object type. Seems to me the most elegant way to deal with this is to move the setWorkingDuration() to the employee class. Probably like this:

class Employee
{
public:
    // Calculates the salary for this employee.
    // Returns the calculated salary.
    virtual int calculateSalary() = 0;
    // Sets the working duration. Does nothing if the employee is permanent.
    // Returns true if Employee is on a contract, false if permanent.
    virtual bool setWorkingDuration(int time)
    {
        return false;
    }
};

class PermanentEmployee : public Employee
{
    const int salaryPerMonth;
public:
    PermanentEmployee(int sal) : salaryPerMonth(sal) {}

    int calculateSalary()
    {
        return salaryPerMonth;
    }
};

class ContractEmployee : public Employee
{
    const int wagesPerHr;
    int totalHour;
public:
    ContractEmployee(int sal) : wagesPerHr(sal), totalHour(0) {}

    int calculateSalary()
    {
        return wagesPerHr * totalHour;
    }

    bool setWorkingDuration(int time)
    {
        totalHour = totalHour + time;
        return true;
    }
};

class Manager
{
    list<Employee *> ls;
public:
    void assignWorkingHours()
    {
        list<Employee *>::iterator it;
        for(it = ls.begin(); it != ls.end(); it++)
        {
            Employee* emp = *it;
            emp->setWorkingDuration(5);
        }
    }
};

This way, the Manager class doesn't have to know whether the Employee is actually a PermanentEmployee or a ContractEmployee. That is what polymorphism gives you. Generally speaking if you have to use dynamic_cast<>, you may want to take another look at the design and see if you can omit it.

Answer 2

Well, the whole point of subtype polymorphism is to allow concrete subclasses to define their own behavior. What you're doing is matching against the type of object you have, and then specifying behavior dependent upon that. Essentially, you've duplicated the entire point of subtypes, and hence missed it. :)

My suggestion? Delegate this behavior to the object itself (not its manager) as a virtual method on Employee.

Answer 3

Of course you can you dynamic_cast operator as well as enum TypeOfEmployee but the last one has nothing common with polymorphism.

You should think why Manager sets working hours to PermanentEmployee. Is it Ok? Another approach is to expand base class interface Employee with setWorkingDuration virtual method. Then in PermanentEmployee it will do nothing.

Answer 4

The others have already provided the answer (worth acceptance) for typical cases.

Here are some notes for less typical cases:

you can reduce your binary size (number of exported symbols), execution times, count of dynamic allocations, total memory usage, and chance for runtime errors if you can avoid using dynamic types at runtime.

removing virtuals and runtime polymorphism can reduce the size by more than 75%.

similarly, you can use variables to identify types or implementations in simple cases (e.g. your approach 2).

i made a simple test. it used multiple dynamic types (360, specifically), rtti, etc. a dylib of the size 588K was generated.

effectively replacing virtual methods and runtime polymorphism with function pointers brings it down to 130K. that's still 360 classes.

consolidating the implementation to a single class with variables only brought the dylib's size down to 10K.

again, this approach affects much more than binary size.

my point is that this is a good substitution under the right circumstances.

Answer 5

Approach three: keep track of contractors separately.

class Manager {
    typedef list<Employee*> Employees;  // Contains all employees, including contractors.
    typedef list<ContractEmployee*> Contractors;   // Contractors are in this list as well.
    Employees employees;
    Contractors contractors;
public:
    void assignWorkingHour() {
        for(Contractors::iterator it = contractors.begin(); it != contractors.end(); ++it) {
            (*it)->setWorkingDuration(5);
        }
    }

    int calculateEmployeeCost() {
        int totalSalary = 0;
        for (Employees::const_iterator it = employees.begin(); it != employees.end(); ++it) {
            totalSalary += (*it)->calculateSalary();
        }
        return totalSalary;
    }
};