Design Questions

[Sorc.Polgara]

Ok, I have a couple of program design questions that I'd like advice on.

What I'm doing is writing a linked list of employee records, which will insert an employee record into the correct position in the list based on the employee's ID number.

I use two structures, here are the definitions:

struct Employee{
   string name;
   int empID;
   float hourlyPay;

   bool operator==(const Employee& rhs);
   bool operator!=(const Employee& rhs);
   bool operator<(const Employee& rhs);
};

struct Node
{
   Employee info;
   Node *link;
};

I use one class that uses both of these structures, here is the class definition and interface:

class EmployeeList
{
   friend ostream& operator<<(ostream&, const EmployeeList&);

private:
   void copyList(const EmployeeList& otherList);
   void destroyList();
   void initList();

public:
   EmployeeList();
   EmployeeList(const EmployeeList& otherList);
   ~EmployeeList();

   void Insert(const Employee& newItem);
   void Delete(const Employee& deleteItem);
   bool Search(const Employee& searchItem);
   void PrintList();
   int Length();

   const EmployeeList& operator=(const EmployeeList&);
   EmployeeList& operator<<(const Employee&);
   EmployeeList& operator>>(Employee&);

protected:
   int count;
   Node *first;
   Node *last;
};

Now the way I've organized my files is by having a header file named "employeelist.h" and a implementation file named "employeelist.cc".  The header file "employeelist.h" contains the definitions of the two structures and one class definition above.  The implementation file "employeelist.cc" contains the declaration of the the three members in the structure Employee that overload relational operators, and the declaration of the class EmployeeList.

Ok, so to my actual questions now.

1.  Is the way I organized the definition and declaration of the two structures and one class among my files fine?  Or would you recommend organizing them among files in a different way?

2.  Would it be better to just declare the three members that overload relational operators for the structure Employee in the definition itself?

Thanks.

EDIT:  Am I using the words definition/define and declaration/declare incorrectly?

OFF-TOPIC:  Wow, I've learned so much about and recieved a much better understanding of pointers, the use of the keyword this, the use of -> to access members, and a deeper understanding of instantiation of objects from writing this program.

[rabbit]

1. Generally definitions (#define, class setups, struct setups, etc...) go in headers, and code (in this case class specific) go into the sources, though technically nothing is incorrect in the way you are doing it.

2. Not really sure about this, I've not done any operator overloading in a very long while.

IMHO, I think that a binary tree would be better for this type of system.  It provides faster searching, insertion, and deletetion, and is very easily managed.

[Kp]

It's traditional that comparison operators shall be declared as const for both sides.  That is, add const between the close parenthesis and the semicolon for the comparison operators in class Employee.

Is this a class project for learning about linked lists?  If not, I'd suggest using STL to contain your records.  I'd do it like this:

Code Select Expand


/* in employee.h */

#include <string>
#include <map>

class Employee
{
std::string name;
int ID;
float hourlyPay;
bool operator==(const Employee&) const;
bool operator!=(const Employee& that) const
{
return !(*this == that);
}
bool operator<(const Employee&) const;
};

typedef std::map<int, Employee> EmployeeMap;


This largely removes the need for an EmployeeList class.  Just define some methods to iterate over the EmployeeMap to do what you want, and you're done.  STL's map is implemented using a red-black tree, which will give considerably better performance than a singly linked list when dealing with large data sets.

[Sorc.Polgara]

Thanks for the advice, I added the "const" to the definitions of the overloaded relational operators.

The actual assignment description is this. 

So yeah, I'm sure you are correct concerning the efficiency, or rather inefficiency of using a linked list.  However it seems like I'm required to use a sorted/ordered linked list for the assignment, as well as have an Employee record as a struct.

I decided to declare the members of the Employee structure in the actual definition in the file "employeelist.h", so that all that is in the file "employeelist.cc" is the declaration/implementation of the EmployeeList class.

Here is the code in the file "employeelist.h"

Code Select Expand


// Michael Souza
// employeelist.h

#ifndef _EMPLOYEELIST_H_
#define _EMPLOYEELIST_H_

#include <iostream>
#include <string>
using namespace std;

//==================================================================
// Employee structure definition w/ declarations
//==================================================================
struct Employee
{
  string name;
  int empID;
  float hourlyPay;

  // Constructor and default constructor
  Employee(int id = 0, float pay = 0.0, string s = ""):
    empID(id), hourlyPay(pay), name(s){}

  // Overload the output stream operator and declare it's behavior
  friend ostream& operator<<(ostream& osObj, const Employee& emp){
    osObj << "Name:\t" << emp.name << "\nID:\t" << emp.empID
          << "\nPay:\t$" << emp.hourlyPay << " per hour\n\n";

    return osObj;
  }

  // Overload the equality relational operator so that it compares
  // an employee record by the employee's ID number
  bool operator==(const Employee& rhs) const{
    return (this->empID == rhs.empID);
  }

  // Overload the inequality relational operator so that it compares
  // an employee record by the employee's ID number
  bool operator!=(const Employee& rhs) const{
    return !(*this == rhs);
  }

  // Overload the less than relational operator so that it compares
  // an employee record by the employee's ID number
  bool operator<(const Employee& rhs) const{
    return (this->empID < rhs.empID);
  }
};

//==================================================================
// Node structure definition
//==================================================================
struct Node
{
  Employee info;
  Node *link;
};

//==================================================================
// EmployeeList class definition/interface
//==================================================================
class EmployeeList
{
  // Overload the output stream operator
  friend ostream& operator<<(ostream&, const EmployeeList&);

private:
  // Copies another list to the list
  void copyList(const EmployeeList&);

  // Clears/resets the list and class variables
  void destroyList();

  // Initializes the list and class variables
  void initList();

public:
  EmployeeList();                                // Default constructor
  EmployeeList(const EmployeeList&);             // Copy constructor
  ~EmployeeList();                               // Destructor

  // Inserts an employee record into the list in ascending order by ID
  void Insert(const Employee&);

  // Removes an employee record from the list
  void Delete(const Employee&);

  // Looks for a employee record in the list
  bool Search(const Employee&);

  // Outputs all employee records in the list
  void PrintList();

  // Get how many employee records are in the list
  int Length();

  // Overloads the list's assignment operator
  const EmployeeList& operator=(const EmployeeList&);

  // Overloads the behavior of the list's output stream operator to
  // insert a employee record into the list
  EmployeeList& operator<<(const Employee&);

  // Overloads the behavior of the list's input stream operator to
  // remove the first employee record from the list, and set the
  // employee record (right operand) to it
  EmployeeList& operator>>(Employee&);

protected:
  int count;        // keeps a count of the number of employee records
  Node *first;      // points to the first employee record in the list
  Node *last;       // points to the last employee record in the list
};

#endif


Now in the definition of the the struct Employee, I have the constructor and default constructor taken care of all in one...

Code Select Expand


// Constructor and default constructor
Employee(int id = 0, float pay = 0.0, string s = ""):
  empID(id), hourlyPay(pay), name(s){}


I found that doing this allows me to Insert(), Delete(), and Search() for an employee record in the list by passing just the employee record's ID number (int).  However if I don't do it like this, I'm unable to just pass an integer that is the employee's ID number, thus I would have to overload these functions.

What would you do in this case?

[Kp]

I'd say to have a Search(int) method and skip having a Search(const Employee&), since it seems a little weird to construct an Employee just so you can search for one just like it.  Ditto for Delete.

Although default parameters are useful, I dislike having constructors with multiple default parameters.  It's too easy to invoke it incorrectly, particularly if several parameters are of the same type.  I'd prefer to have Employee() and Employee(int, float, string) as separate constructors.  Employee() should zero out the numeric fields.  Employee(int, float, string) should be just like what you have now (except not using default parameters).

With what you have now, I could create an Employee with an ID and a salary, but no name by inserting Employee(5, .230 /* and a default name of "" */).  With the changes I propose, such a declaration would be an error - you'd have to explicitly specify the blank name if that's what you wanted.