Homework help: is this a good design?
CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Page 1 of 2 12 LastLast
Results 1 to 15 of 19

Thread: Homework help: is this a good design?

Hybrid View

  1. #1
    Join Date
    Dec 2013
    Posts
    75

    Homework help: is this a good design?

    Hi guys,

    I'd like some advice on my C++ homework, which is to create an Employee class with three data members (first name, last name, and salary), accessor methods for each, and a function to display all three data members. My instructor isn't very picky about how we implement his requirements, just that it gives the results he's looking for.

    Here's my design:

    Employee.hpp
    Code:
    //inclusion guards
    #ifndef EMPLOYEE_HPP
    #define EMPLOYEE_HPP
    
    //including string header file
    #include <string>
    using std::string;
    
    //Employee class declaration
    class Employee
    {
    public:
    //ctor
    Employee(string fn,string ln,double s);
    //cctor
    Employee(const Employee& rhs);
    //move ctor
    Employee(Employee&& rhs);
    //copy assignment operator
    virtual Employee& operator=(const Employee& rhs);
    //move assignment operator
    virtual Employee& operator=(Employee&& rhs);
    //~dtor
    virtual ~Employee();
    //getters and setters
    virtual string& GetFirstName() const;
    virtual void SetFirstName(const string& fn);
    virtual string& GetLastName() const;
    virtual void SetLastName(const string& ln);
    virtual double& GetSalary();
    virtual void SetSalary(const double& s);
    //fn to display Employee data
    virtual void DisplayEmployee() const;
    private:
    struct EmployeeData;
    EmployeeData* data;
    };
    #endif
    Employee.cpp
    Code:
    //headers for string and I/O
    #include <iostream>
    #include <string>
    using std::cout;
    using std::endl;
    using std::string;
    
    
    //include Employee class declaration
    #include "Employee.hpp"
    
    //struct to hold Employee data
    struct EmployeeData
    {
    EmployeeData(string fn,string ln,double s);
    EmployeeData(const EmployeeData& rhs)=default;
    EmployeeData& operator=(const EmployeeData& rhs)=default;
    string firstname;
    string lastname;
    double salary;
    };
    
    
    //EmployeeData ctor definition
    EmployeeData::EmployeeData(string fn,string ln,double s)
    {
    if (s < 0)
    salary=0.00;
    else
    {
    firstname=fn;
    lastname=ln;
    salary=s;
    }
    }
    
    //Employee ctor definition
    Employee::Employee(string fn = "unknown",string ln = "unknown",double s = 0.0):data(new EmployeeData(fn,ln,s))
    {
    }
    
    //Employee cctor def
    Employee::Employee(const Employee& rhs):data(new EmployeeData(rhs.*data))
    {
    }
    
    //Employee move ctor def
    Employee::Employee(Employee&& rhs):data(new EmployeeData(rhs.*data))
    {
    //null rhs's data ptr to prevent needless memory deallocation when rhs goes out of scope
    rhs.data=0;
    }
    
    //Employee copy assignment operator def
    virtual Employee& Employee::operator=(const Employee& rhs)
    {
    //check for self-assignment
    if(this==&rhs)
    return this;
    else
    this.*data=rhs.*data;
    return this;
    }
    
    //Employee move assignment operator def
    virtual Employee& Employee::operator=(Employee&& rhs)
    {
    //check for self-assignment
    if(this==&rhs)
    return this;
    else
    this.*data=rhs.*data;
    //null rhs's data ptr to prevent needless memory deallocation when rhs goes out of scope
    rhs.data=0;
    return this;
    }
    
    //Employee dtor def;
    virtual Employee::~Employee()
    {
    if(data) //only deallocate memory if data is a null ptr
    delete data;
    }
    
    virtual string& Employee::GetFirstName() const
    {
    return data->firstname;
    }
    
    virtual void Employee::SetFirstName(const string& fn)
    {
    data->firstname=fn;
    }
    
    virtual string& Employee::GetLastName() const
    {
    return data->lastname;
    }
    
    virtual void Employee::SetLastName(const string& ln)
    {
    data->lastname=ln;
    }
    
    virtual double& Employee::GetSalary() const
    {
    return data->salary;
    }
    
    virtual void Employee::SetSalary(const double& s)
    {
    if(s >= 0)
    data->salary=s;
    else
    data->salary=0.0;
    }
    
    virtual void Employee::DisplayEmployee() const
    {
    cout << "First Name: " << /t << data->firstname << endl;
    cout << "Last Name: " << /t << data->lastname << endl;
    cout << "Salary: " << /t << data->salary << endl;
    }
    Driver.cpp
    Code:
    /include iostream and string headers
    #include <iostream>
    #include <string>
    using std::cout;
    using std::cin;
    using std::endl;
    using std::string;
    
    //include Employee class def
    #include "Employee.hpp"
    
    //begin driver
    int main()
    {
    
    //variables for employee's first and last names and salary
    string FirstName;
    string LastName;
    double salary;
    
    //read user input
    cout << "Enter an employee's first and last name, pressing /"enter/" after each name." << endl;
    cin >> FirstName >> LastName;
    cout << "Enter the employee's salary." << endl;
    cin >> salary;
    
    //create an Employee using the input
    Employee officedrone(FirstName,LastName,salary);
    
    //display the Employee's fields
    officedrone.DisplayEmployee();
    
    //ask user to modify the Employee's salary
    cout << officedrone.GetFirstName() << " " << officedrone.GetLastName() << " has received a promotion!"
    << "Please enter his new salary: " << endl;
    cin >> salary;
    
    officedrone.SetSalary(salary);
    
    cout << officedrone.GetFirstName() << " " << officedrone.GetLastName() << " has decided to use his newfound income to get a sex change! What name should she now be known as?" << endl;
    cin >> FirstName >> LastName;
    
    officedrone.SetFirstName(FirstName);
    officedrone.SetLastName(LastName);
    
    //display the modified Employee's fields
    officedrone.DisplayEmployee();
    
    //All done! Return to OS!
    return 0;
    }
    The assignment is to demonstrate our understanding of basic OOP principles.

    Also, wish me luck on my C++ test tonight!

  2. #2
    GCDEF is offline Elite Member Power Poster
    Join Date
    Nov 2003
    Posts
    12,046

    Re: Homework help: is this a good design?

    The only slight niggle that jumps out at me right away is that you've made all your functions virtual. In the case of getting and setting the names and probably the salary, that doesn't seem necessary.

  3. #3
    Join Date
    Dec 2013
    Posts
    75

    Re: Homework help: is this a good design?

    He told us we'll be inheriting from Employee in later assignments.

  4. #4
    GCDEF is offline Elite Member Power Poster
    Join Date
    Nov 2003
    Posts
    12,046

    Re: Homework help: is this a good design?

    Quote Originally Posted by TSLexi View Post
    He told us we'll be inheriting from Employee in later assignments.
    That doesn't mean all your functions need to be virtual. Only make them virtual if you think their behavior will change in derived classes. GetFirstName would be the same regardless of the employee class I would think, therefore you'll want the behavior of the parent class.

  5. #5
    Join Date
    Dec 2013
    Posts
    75

    Re: Homework help: is this a good design?

    Also, should my functions pass arguments by value, by const lvalue ref, or by rvalue ref?

  6. #6
    Join Date
    Jul 2013
    Posts
    200

    Re: Homework help: is this a good design?

    Quote Originally Posted by TSLexi View Post
    Also, should my functions pass arguments by value, by const lvalue ref, or by rvalue ref?
    The choise between by value and by const reference is a question of efficiency alone. Pass by value involves a copy of the parameter to be produced and the more this costs the more attractive pass by const reference becomes. The "cost" is not only the size of the data but also other calculations that may be performed (such as for example incrementing the counter in case of a smart pointer).

    A common rule of thumb is that primitives are passed by value and everything more expensive is by const reference. Accordingly a double should be passed by value and a string by const reference.
    Last edited by razzle; February 28th, 2014 at 07:34 AM.

  7. #7
    Join Date
    Apr 2000
    Location
    Belgium (Europe)
    Posts
    3,756

    Re: Homework help: is this a good design?

    that question depends entirely on what your expectancies are.

    there are reasons for all 3, and reasons why any of the 3 could be inappropriate in a given context.

    typically speaking though:
    If you can pass by const reference and this is the behaviour you want, then do so
    if you need to change the object, use a non-const reference.
    only use pass by value if you explicitely want this to be what's happening (= operating on a copy)
    only use pass by rvalue ref if you intend to "move ownership" of members.

    you forgot about pass by pointer, which is good. pointers are "bad". only use them when none of the above is adequate.

  8. #8
    Join Date
    Apr 1999
    Posts
    27,422

    Re: Homework help: is this a good design?

    Code:
    //Employee dtor def;
    virtual Employee::~Employee()
    {
        // if(data) no need for this
            delete data;
    }
    There is no need to check if data is not null to delete it. It is perfectly safe to call delete on a null pointer.

    Regards,

    Paul McKenzie

  9. #9
    Arjay's Avatar
    Arjay is offline Moderator / MS MVP Power Poster
    Join Date
    Aug 2004
    Posts
    11,195

    Re: Homework help: is this a good design?

    Quote Originally Posted by Paul McKenzie View Post
    Code:
    //Employee dtor def;
    virtual Employee::~Employee()
    {
        // if(data) no need for this
            delete data;
    }
    There is no need to check if data is not null to delete it. It is perfectly safe to call delete on a null pointer.

    Regards,

    Paul McKenzie
    Similarly.. be sure you always initialize the data to null in the constructor.

  10. #10
    GCDEF is offline Elite Member Power Poster
    Join Date
    Nov 2003
    Posts
    12,046

    Re: Homework help: is this a good design?

    Another thing, in most cases functions don't want to return references. That applies here.

  11. #11
    Join Date
    Apr 1999
    Posts
    27,422

    Re: Homework help: is this a good design?

    Quote Originally Posted by TSLexi View Post
    Hi guys,

    I'd like some advice on my C++ homework, which is to create an Employee class with three data members (first name, last name, and salary),
    Personally I wouldn't return references in your class. Just return the data.

    The reason is that first, the data in your class represent "simple" things -- a salary, a name, etc. The data isn't a complex object, where it doesn't make sense for that object to survive beyond its containing object. So for things such as strings, integers, double, etc. return the data.

    Another reason is that if that Employee object goes out of scope and your code is holding onto one of the values that was returned by reference, you are now referencing an invalid string. You now have a bug in your program, and for what reason? Again, return the data, not a reference, and then you will be ok.

    Regards,

    Paul McKenzie

  12. #12
    Join Date
    Jul 2013
    Posts
    200

    Re: Homework help: is this a good design?

    Quote Originally Posted by Paul McKenzie View Post
    Personally I wouldn't return references in your class. Just return the data.
    Yet another aspect is design.

    The only reason for the use of getter/setter methods is to increase the level of encapsulation by hiding implementation. When the getter is returning a reference to internal data that motivation is gone. The getter/setter serves no purpose any longer and could as well be removed.
    Last edited by razzle; February 28th, 2014 at 07:38 AM.

  13. #13
    Join Date
    Dec 2013
    Posts
    75

    Re: Homework help: is this a good design?

    Well, the instructor required us to write getter and setter methods for Employee's data members.

    Also, as of yet, we have no idea if the implementation of Employee will significantly change in its derived classes, and our compiler can devirtualize method calls.
    Last edited by TSLexi; February 28th, 2014 at 02:25 PM.

  14. #14
    Join Date
    Dec 2013
    Posts
    75

    Re: Homework help: is this a good design?

    OK, here's the final version (I modified the move ctor so it just switches the pointers around):

    Employee.hpp
    Code:
    //inclusion guards
    #ifndef EMPLOYEE_HPP
    #define EMPLOYEE_HPP
    
    //including string header file
    #include <string>
    using std::string;
    
    class EmployeeData;
    //Employee class declaration
    class Employee
    {
    public:
    //ctor
    Employee(string fn,string ln,double s);
    //cctor
    Employee(const Employee& rhs);
    //move ctor
    Employee(Employee&& rhs);
    //copy assignment operator
    virtual Employee operator=(const Employee& rhs);
    //move assignment operator
    virtual Employee operator=(Employee&& rhs);
    //~dtor
    virtual ~Employee();
    //getters and setters
    virtual string GetFirstName()const;
    virtual void SetFirstName(const string& fn);
    virtual string GetLastName()const;
    virtual void SetLastName(const string& ln);
    virtual double GetSalary() const;
    virtual void SetSalary(const double& s);
    //fn to display Employee data
    virtual void DisplayEmployee() const;
    private:
    EmployeeData* data;
    };
    #endif
    Employee.cpp
    Code:
    //headers for string and I/O
    #include <iostream>
    #include <iomanip>
    #include <string>
    using std::cout;
    using std::endl;
    using std::setprecision;
    using std::fixed;
    using std::string;
    
    
    //include Employee class declaration
    #include "Employee.hpp"
    
    //struct to hold Employee data
    class EmployeeData
    {
    public:
    EmployeeData(string fn,string ln,double s);
    EmployeeData(const EmployeeData& rhs)=default;
    EmployeeData& operator=(const EmployeeData& rhs)=default;
    string firstname;
    string lastname;
    double salary;
    };
    
    
    //EmployeeData ctor definition
    EmployeeData::EmployeeData(string fn,string ln,double s)
    {
    firstname=fn;
    lastname=ln;
    if(salary < 0)
    salary=0.0;
    else
    salary=s;
    }
    
    //Employee ctor definition
    Employee::Employee(string fn = "unknown",string ln = "unknown",double s = 0.0):data(new EmployeeData(fn,ln,s))
    {
    }
    
    //Employee cctor def
    Employee::Employee(const Employee& rhs):data(new EmployeeData(*(rhs.data)))
    {
    }
    
    //Employee move ctor def
    Employee::Employee(Employee&& rhs):data(0)
    {
    data=rhs.data;
    //null rhs's data ptr to prevent needless memory deallocation when rhs goes out of scope
    rhs.data=0;
    }
    
    //Employee copy assignment operator def
    Employee Employee::operator=(const Employee& rhs)
    {
    //check for self-assignment
    if(this==&rhs)
    return *this;
    else
    *(this->data)=*(rhs.data);
    return *this;
    }
    
    //Employee move assignment operator def
    Employee Employee::operator=(Employee&& rhs)
    {
    //check for self-assignment
    if(this==&rhs)
    return *this;
    delete data;
    data=rhs.data;
    //null rhs's data ptr to prevent needless memory deallocation when rhs goes out of scope
    rhs.data=0;
    return *this;
    }
    
    //Employee dtor def;
    Employee::~Employee()
    {
    if(data) //only deallocate memory if data is a valid ptr
    delete data;
    }
    
    string Employee::GetFirstName()const
    {
    return data->firstname;
    }
    
    void Employee::SetFirstName(const string& fn)
    {
    data->firstname=fn;
    }
    
    string Employee::GetLastName()const
    {
    return data->lastname;
    }
    
    void Employee::SetLastName(const string& ln)
    {
    data->lastname=ln;
    }
    
    double Employee::GetSalary()const
    {
    return data->salary;
    }
    
    void Employee::SetSalary(const double& s)
    {
    if(s >= 0)
    data->salary=s;
    else
    data->salary=0.0;
    }
    
    void Employee::DisplayEmployee()const
    {
    cout << "First Name: " << data->firstname << endl;
    cout << "Last Name: "  << data->lastname << endl;
    cout << "Salary: " << fixed << setprecision(2) << data->salary << endl;
    }
    Driver.cpp
    Code:
    //include iostream and string headers
    #include <iostream>
    #include <string>
    using std::cout;
    using std::cin;
    using std::endl;
    using std::string;
    
    //include Employee class def
    #include "Employee.hpp"
    
    //begin driver
    int main()
    {
    
    //variables for employee's first and last names and salary
    string FirstName;
    string LastName;
    double salary;
    
    //read user input
    cout << "Enter an employee's first and last name, pressing <" << "enter" << "> after each name." << endl;
    cin >> FirstName >> LastName;
    cout << "Enter the employee's salary." << endl;
    cin >> salary;
    
    //create an Employee using the input
    Employee officedrone(FirstName,LastName,salary);
    
    //display the Employee's fields
    officedrone.DisplayEmployee();
    
    //ask user to modify the Employee's salary
    cout << officedrone.GetFirstName() << " " << officedrone.GetLastName() << " has received a promotion!"
    << "Please enter his new salary: " << endl;
    cin >> salary;
    
    officedrone.SetSalary(salary);
    
    cout << officedrone.GetFirstName() << " " << officedrone.GetLastName() << " has decided to use his newfound income to get a sex change! What name should she now be known as?" << endl;
    cin >> FirstName >> LastName;
    
    officedrone.SetFirstName(FirstName);
    officedrone.SetLastName(LastName);
    
    //display the modified Employee's fields
    officedrone.DisplayEmployee();
    
    //All done! Return to OS!
    return 0;
    }

  15. #15
    Join Date
    Jul 2013
    Posts
    200

    Re: Homework help: is this a good design?

    Quote Originally Posted by TSLexi View Post
    OK, here's the final version
    A final touch would be to remove everything virtual.

    As newbie (and well beyond that) you only have to concern yourself with how to implement two types of classes cleanly. These are value classes and OO polymorphic classes.

    Value classes are modelled after the primitives like int and double and include most of the standard library classes such as std::string and std:vector. They all have copy semantics and they're not intended to be inherited from.

    As it's implemented, Employee is a typical value class. Using virtual methods doesn't make it an OO polymorphic class. It's just bogus marketing.

    So make Employee a proper value class for this assignment. Then in later assigments (when Employee is to be inherited from) change the design to a proper OO polymorphic class. It won't have copy semantics and it won't carry state (data).

    Finally note that your implementation of copy sematics for the Employee class becomes much simpler if Employee would hold the EmployeeData object by value rather than by pointer. It's because all members of EmployeeData ara themselves value classes which means the default copy machinery of Employee (the default copy constructor and the default copy assignment operator) will then automatically supply the copy semantics. It's like for free. But it's much less of an exercise of course.
    Last edited by razzle; February 28th, 2014 at 06:59 AM.

Page 1 of 2 12 LastLast

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  


Azure Activities Information Page

Windows Mobile Development Center


Click Here to Expand Forum to Full Width

This is a CodeGuru survey question.


Featured


HTML5 Development Center