HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Single inheritance polymorphism

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
inheritancepolymorphismsingle

Problem

I am learning about polymorphism in C++ and decided to implement an example. Is there anything I need to fix in my code? This compiles and runs.

#include 

using std::cout;
using std::endl;
using std::cin;

class Employee
{
public:
    Employee():salary(1000){}
    virtual ~Employee(){}
    virtual void Talk() const {cout > choice;
        if(choice == 1)
            pEmployee = new Programmer;
        else
            pEmployee = new SalesPerson;

        Company[i] = pEmployee;
    }

    cout Talk();

        Programmer *pCoder = dynamic_cast (Company[i]);

        if(pCoder)
            pCoder->Compiling();

        cout << endl;
    }

    return 0;
}

Solution

Problems with your interface design:

The Employee interface you've designed is clearly lacking
abstraction, since you have a dynamic_cast in the program to
test if an employee is of a specific type.

Employee should instead have a virtual method such as DoWork()
which in turn will call Compiling() for Programmer,
Selling for a SalesPerson and so forth:

class Employee 
{
public:
    ....

    // Virtual function with no default implementation (= 0)
    virtual void DoWork() const = 0;
};

class Programmer 
{
public:
    void DoWork() const { Compiling(); }
};

class SalesPerson
{
public:
    void DoWork() const { Selling(); }
};


The constructor of Employee sets salary to 1000 unconditionally.
Perhaps it would be better to take salary as a constructor parameter
so you can easily specify different salaries:

Employee(double salary) : salary(salary) { }


BTW, double seems more appropriate for a monetary value, as it
is likely to include decimal places. int is for whole numbers only.

The destructor of Employee is empty, so it should be made a default:

virtual ~Employee() = default;


Memory management tidbits:

It is generally better to avoid manual memory management, as it is
error prone, not exception safe, and slipping a memory leak is very easy.
In fact, in your current example, the objects you've allocated
with new are not being destroyed. Remember that C++ is not garbage collected. You would have to delete them somewhere.

You should ideally be using an automatic pointer to manage
your dynamic instances. main() could be reworked in the following way:

#include 
#include 

int main()
{
    std::array, 3> company;

    int choice = 0;
    for (int i = 0; i > choice;

        if (choice == 1)
        {
            company[i].reset( new Programmer );
        }
        else
        {
            company[i].reset( new SalesPerson );
        }
    }

    ....
}


You should note in the example above the use of std::unique_ptr and
std::array. The unique_ptr is a managed pointer that will delete
the object automatically when you leave the function scope. This way
you don't have to worry about memory leaks. std::array is a fixed size
array which behaves just like a plain C array, but with some extra member
methods and debug bounds checking. It also knows its length, so we no longer
need to keep that numEmployees constant around.

Code Snippets

class Employee 
{
public:
    ....

    // Virtual function with no default implementation (= 0)
    virtual void DoWork() const = 0;
};

class Programmer 
{
public:
    void DoWork() const { Compiling(); }
};

class SalesPerson
{
public:
    void DoWork() const { Selling(); }
};
Employee(double salary) : salary(salary) { }
virtual ~Employee() = default;
#include <array>
#include <memory>

int main()
{
    std::array<std::unique_ptr<Employee>, 3> company;

    int choice = 0;
    for (int i = 0; i < company.size(); i++)
    {
        std::cout << "(1) Programmer (2) SalesRep: ";
        cin >> choice;

        if (choice == 1)
        {
            company[i].reset( new Programmer );
        }
        else
        {
            company[i].reset( new SalesPerson );
        }
    }

    ....
}

Context

StackExchange Code Review Q#64846, answer score: 6

Revisions (0)

No revisions yet.