Hello, OnlineGDB Q&A section lets you put your programming query to fellow community users. Asking a solution for whole assignment is strictly not allowed. You may ask for help where you are stuck. Try to add as much information as possible so that fellow users can know about your problem statement easily.

Let's get better in C++ - Exercise #5

+13 votes
asked Mar 4 by Peter Minarik (32,920 points)
edited Mar 8 by Peter Minarik

Update: A Bit of Guidance

I thought I'll provide a bit of help for this little challenge. I suspect people are looking at the problem from a wrong angle.

So here are a few more clues to solve this:

  1. Imagine, that one would need to change how the names are stored. For instance, one would like to store the first name and last name separately. Also store middle name(s) too, if any. Also, title would be needed to be stored. These shouldn't go inti a single string as now, but into multiple fields.
  2. Now how much effort would it be to implement this?
  3. How could have this been way less problem if the classes were designed differently?
  4. You can have the same thought process for the ID as well. What is we wanted it to contain letters too? How many times do we need to change this? How could have we done things better in the design to only need to change the ID one place only?

I hope this helps to see the flow of the design of the code below. Share you idea how it could have been done better. ;)

Instructions

This exercise is for anyone who would like to get better in object-oriented design.

I've provided a sample code, that has a problems with it. (I was focusing on one specific problem, however, other mistakes could have been made.) The sample code could have been written in C# or Python or Java or any object oriented language. In other words, look for design problems, not syntax or C++ specific errors. 

Let's see who finds the problem.

Please, tell me

  1. what part of the code has a problem
  2. why it is a problem
  3. how would you fix it

Notes & Tips

  • The code compiles and runs (using C++ compilation language)
  • Do not get discouraged by the length. Just read though, try to understand what we're trying to do here and find what potential issue the implementation has.

The Code

The whole code can be downloaded from here, or just read below:

main.cpp

#include <iostream>

#include "Manager.h"

static void PrintHierarchy(const Company::Manager * manager)
{
    std::cout << manager->GetName() << std::endl;
    for (Company::TeamLead * teamLead : manager->GetTeamLeads())
    {
        std::cout << '\t' << teamLead->GetName() << std::endl;
        for (Company::Programmer * programmer : teamLead->GetProgrammers())
        {
            std::cout << "\t\t" << programmer->GetName() << std::endl;
        }
    }
}

int main()
{
    Company::Manager manager("Charles Manager", 0);
    Company::TeamLead teamLead_A("Alpha Lead", 1);
    Company::TeamLead teamLead_B("Bravo Lead", 2);
    Company::Programmer programmer_A0("John Alpha", 3);
    Company::Programmer programmer_A1("Jane Alpha", 4);
    Company::Programmer programmer_A2("Jeffrey Alpha", 5);
    Company::Programmer programmer_B0("Amos Bravo", 6);
    Company::Programmer programmer_B1("Ane Bravo", 7);
    Company::Programmer programmer_B2("Amy Bravo", 8);
    Company::Programmer programmer_B3("Arthur Bravo", 9);
    
    manager.Add(&teamLead_A);
    manager.Add(&teamLead_B);
    
    teamLead_A.Add(&programmer_A0);
    teamLead_A.Add(&programmer_A1);
    teamLead_A.Add(&programmer_A2);
    
    teamLead_B.Add(&programmer_B0);
    teamLead_B.Add(&programmer_B1);
    teamLead_B.Add(&programmer_B2);
    teamLead_B.Add(&programmer_B3);
    
    PrintHierarchy(&manager);

    return 0;
}

Programmer.h

#pragma once

#include <string>

namespace Company
{
    class Programmer
    {
    private:
        std::string _name;
        unsigned int _id;
        float _hoursWorked;

    public:
        Programmer(std::string name, unsigned int _id);
        const std::string & GetName() const;
        unsigned int GetId() const;
        void Work(float hours);
    };
}

Programmer.cpp

#include "Programmer.h"

namespace Company
{
    Programmer::Programmer(std::string name, unsigned int id) :
        _name(name),
        _id(id),
        _hoursWorked(0.0f)
    { }
    
    const std::string & Programmer::GetName() const { return _name; }
    
    unsigned int Programmer::GetId() const { return _id; }
    
    void Programmer::Work(float hours) { _hoursWorked += hours; }
}

TeamLead.h

#pragma once

#include <string>
#include <vector>

#include "Programmer.h"

namespace Company
{
    class TeamLead
    {
    private:
        std::vector<Programmer *> _programmers;
        std::string _name;
        unsigned int _id;
        float _hoursWorked;

    public:
        TeamLead(std::string name, unsigned int _id);
        const std::string & GetName() const;
        unsigned int GetId() const;
        void Work(float hours);
        void Add(Programmer * programmer);
        const std::vector<Programmer *> & GetProgrammers() const;
    };
}

TeamLead.cpp

#include "TeamLead.h"

namespace Company
{
    TeamLead::TeamLead(std::string name, unsigned int id) :
        _name(name),
        _id(id),
        _hoursWorked(0.0f)
    { }
    
    const std::string & TeamLead::GetName() const { return _name; }
    
    unsigned int TeamLead::GetId() const { return _id; }
    
    void TeamLead::Work(float hours) { _hoursWorked += hours; }
    
    void TeamLead::Add(Programmer * programmer) { _programmers.push_back(programmer); }
    
    const std::vector<Programmer *> & TeamLead::GetProgrammers() const { return _programmers; }
}

Manager.h

#pragma once

#include <string>
#include <vector>

#include "TeamLead.h"

namespace Company
{
    class Manager
    {
    private:
        std::vector<TeamLead *> _teamLeads;
        std::string _name;
        unsigned int _id;
        float _hoursWorked;

    public:
        Manager(std::string name, unsigned int _id);
        const std::string & GetName() const;
        unsigned int GetId() const;
        void Work(float hours);
        void Add(TeamLead * teamLead);
        const std::vector<TeamLead *> & GetTeamLeads() const;
    };
}

Manager.cpp

#include "Manager.h"

namespace Company
{
    Manager::Manager(std::string name, unsigned int id) :
        _name(name),
        _id(id),
        _hoursWorked(0.0f)
    { }
    
    const std::string & Manager::GetName() const { return _name; }
    
    unsigned int Manager::GetId() const { return _id; }
    
    void Manager::Work(float hours) { _hoursWorked += hours; }
    
    void Manager::Add(TeamLead * teamLead) { _teamLeads.push_back(teamLead); }
    
    const std::vector<TeamLead *> & Manager::GetTeamLeads() const { return _teamLeads; }
}

4 Answers

+1 vote
answered Mar 15 by Peter Minarik (32,920 points)
 
Best answer

Update 3

Unfortunately no one came along and provided an implementation utilizing inheritance to allow common functionality to be handled in one place. Without this, the code contains lots of repeated (copy-pasted) parts that increase maintenance costs.

Let me provide a possible solution to the original problem, in case someone stumbles on this problem in the future.

+3 votes
answered Mar 4 by xDELLx (8,580 points)
One thing which looks "wrong" ,is the vectors hold memory addresses of leads/programers.

Its possible that a local variable of Team Lead/Programmer  was used and later the function exists ,clearing its local variables. If during this time ,the local variable was added to one of the vectors & was not cleared ,accessing it could crash the program. :)

I would suggest to store the copies of employees in the vectors.

I wil add more to this list later.
commented Mar 5 by Peter Minarik (32,920 points)
Yes, you are right. :)

It's a potential source of problem, though all the instances have the same scope, so going out of scope happens for all of them, so I don't believe this problem actually affects the code in this particular scenario. After all, everything is in the main, when it exits, the program closes.

Also, I could have used std::shared_ptr, but I didn't want to complicate the code. I tried to keep things as simple as possible while demonstrating a faulty design.

The instruction hints it: "This exercise is for anyone who would like to get better in object-oriented design."

I was trying to get people engaged and see if they can see a huge problem with the design of the classes. I deliberately made the design flawed and I'm trying to make people see this problem so they wouldn't do the same mistake when they design their own classes.

So the question is still up: what's wrong with the design of the classes? What's wrong with the design of the little exercise program?
0 votes
answered Mar 10 by Peter Minarik (32,920 points)

Update 2: I'll Tell You The Problem, You Give Me The Fix :)

Okay, maybe I need to push you guys a bit more.

I'll Show The Issue

As described in the previous update (see below), we have a problem if we want to change something, like the name or the ID. Why is that?

Well, if you need to change the name, you have to change it in 3 different classes:

  • Programmer
  • TeamLead
  • Manager

This is too much unnecessary work. If you think about all of the above classes represent a person. One could have realized this at the design and extracted a common base for all of these classes: Employee. An Employee would have all that these share: nameIDhoursWorked.

Similarly, the TeamLead and the Manager is built up alike: they have a series of subordinates. So a common base could have been extracted for these classes as well.

You Do The Implementation 

Now that we've pointed out what exactly the design flow was, I'm waiting for the implementations -- using inheritance as it should have been done in the first place.

Good luck! :)

0 votes
answered Jul 30 by Alex (180 points)
it seems there is a problem with (#include "Manager.h"
commented Jul 30 by Peter Minarik (32,920 points)
How do you mean?

What problem do you think there is with including the Manager header file?
commented Jul 30 by Alex (180 points)
maybe u can help coz #include "manager.h" or #include <graphics> is not working
commented Jul 30 by Peter Minarik (32,920 points)
edited Jul 30 by Peter Minarik
I do not have any headers called "graphics" or "manager.h".

My header file is called "Manager.h". Header files are case sensitive. (Especially on Unix based operating systems.)

Also, if you click on the link https://www.onlinegdb.com/BklguaH7_, you should be able to see the code working.
Welcome to OnlineGDB Q&A, where you can ask questions related to programming and OnlineGDB IDE and and receive answers from other members of the community.
...