From C#/Java to C++: Understanding C++ references in an example?

Question

Coming from C#/Java background and I'm trying to understand best practices for using C++ pointers and references. I'm sure this has been covered ad nauseum on this site, but I still don't fully understand it. I've read some of the C++ FAQ but I need to see it in context. Suppose I have two classes:

class Employee
{
  Employee();
  ~Employee();
}


class Company
{
   Company();
   ~Company();

   AddEmployee(?? employee);
   ?? GetEmployee();

private:
   std::list<Employee??> employees_;
}

Where AddEmployee takes an employee object and adds it to the private list of employees. What should the type of the AddEmployee employee parameter be?

AddEmployee(Employee *employee);
AddEmployee(Employee& employee);

What template type should the private std::list be?

Also assuming the company class can modify the employee is const appropriate here? And if I use a reference parameter, can the Company class turn around and pass the employee object to another class?

UPDATE

I see a lot of people suggesting using value semantics but coming from C#/java this is challenging. Generally I'm doing dependency injection using interfaces and I'm not sure if you can even use value semantics in this case. Can you code to interfaces using value semantics?

In the code I'm working on now pretty much all objects are allocated on the heap and my member functions looks like void TakeSomeObject(SomeObject *someObject) or MyClass(SomeService *service). The code works although as the code grows more complicated I am worried about memory leaks as well. Part of what I was wondering if I should change those method signatures to use references instead and what the implications of doing that are.

Also when I read about smart pointers I wanted to go through my code and change most pointers to shared_ptr but it's not clear to me if this is a good thing or not. And in that case would I just change all my methods to take shared_ptr as parameter?

UPDATE 2

It seems like the consensus is to use either AddEmployee(const Employee& employee) or AddEmployee(Employee& employee) for heap allocated Employees. I'm open to change in the face of strong evidence but I don't see myself writing the value semantics version right now: AddEmployee(Employee employee)

UPDATE 3

I tried writing a method like:

AddEmployee(const Employee& employee)
{
  employees_.push_back(??employee);
}

However this doesn't seem to work trying to replace ?? with nothing, &, or * when the list as defined as std::list<Employee*> employees_. Does this mean I have to take a pointer parameter rather than a reference parameter to do something like this?

Answer 1

What should the type of the AddEmployee employee parameter be?

The parameter should be of type Employee const&, that is a const reference to an employee. This is to avoid a copy as the parameter argument, and to specify that the referred object won't be modified from within the function. That in turn allows the function to be called with temporary values.

What template type should the private std::list be?

The type of list should be std::list< Employee >, and the class Company would be the owner of such Employee objects (well actually the list will). If you are looking at more complex ownership relationships, like the ones one usually finds in C#, you'd be better of with a container of smart pointers (or a pointer container from Boost).

Answer 2

The answer to your first two parts (about AddEmployee and GetEmployee depend in part on how much copying Employee hurts. Is it copyable at all? If not, then you have to pass it by reference.

If it is copyable, do you want people to copy it around easily? Is it just a small bundle of simple types, or does it contain heavier objects (strings, arrays, etc)? If it's fairly bulky, then you want to copy as little as possible, so passing by reference is reasonable.

Now for AddEmployee, there is really no reason not to take it as a const& value. You're going to have to copy it to put it in the std::list (more on that later), so there's no real problem with taking it as a constant reference. This can be as const Employee & or Employee const &; take your pick, they mean the same.

For GetEmployee, you should be asking yourself a question: what do I want the user to do with it? For example, in some cases, you want the user to be able to change the actual Employee object stored in the std::list. Now granted, I would consider that slightly suspect, or at least worthy of some form of explanation of why you need to give your actual Employee object to someone to change, but it is fairly common to return a reference.

If you do, you should also provide a const version of GetEmployee that returns a const&. Like so:

Employee &GetEmployee();
const Employee &GetEmployee() const;

That way, if they have a const Company, they can get a const Employee that they can look at, but not modify.

As for std::list, that's really simple: you never store a container of references. EVER. Your options are values or pointers. Actually, your options really should be values or smart pointers (look those up). Since we're not dealing with smart pointers here, we should just use values.

So I would suggest this:

class Company
{
   Company();
   ~Company();

   AddEmployee(const Employee &employee);
   const Employee &GetEmployee() const;
   Employee &GetEmployee();

private:
   std::list<Employee> employees_;
}

Answer 3

What should the type of the AddEmployee employee parameter be?

Depends on what you want to do. If you want to give up ownership of the allocated object, then you should use a pointer. However, pointers are not guaranteed to be assigned or valid, so it is more risky to take in an employee of this type.

// Example where you give up ownership
Employee* employee = new Employee();

// Call the method taking in a pointer to an employee
AddEmployee(employee);

// You allocated it, but you didn't delete it,
// so whoever owns employee now will have to delete it.
employee = NULL; 

You can be bad about it too:

// Example where you give up ownership
Employee* employee = NULL;

// Oops, you're passing in a null pointer.
AddEmployee(employee);

// Allocate an employee
employee = new Employee(); 
delete employee;

// Oops, you're passing in an deallocated object which is 
// the worst case scenario because the behavior is undefined.
AddEmployee(employee);

Taking in a reference to an Employee object is safer, because the language does not allow you to pass a null reference: in other words the reference is guaranteed to be allocated and non-null. It is recommended that you use a reference instead of a pointer, whenever it makes sense to do so.

AddEmployee(Employee& employee);

---

What template type should the private std::list be?

Update: I get your question now and the answer is complicated: although for your specific case, it would probably be better to have pointers to Employee since they will most likely be long-lived objects and you don't want to store those on the stack. So for your case, the answer is list<Employee*>. However, here are a few questions that address many of the nuances involved in deciding when to go with stack allocated vs heap allocated:

• Which is faster: Stack allocation or Heap allocation
• What and where are the stack and heap?
• About C/C++ stack allocation
• When is stack space allocated for local variables?
• C++ STL: should I store entire objects, or pointers to objects?

---

Also assuming the company class can modify the employee is const appropriate here?

If you use a reference and you don't copy the object, then you can't pass it in as const and modify it. If you use a pointer, then you get into const correctness and you can read more abut it here: http://www.parashift.com/c++-faq-lite/const-correctness.html

---

And if I use a reference parameter, can the Company class turn around and pass the employee object to another class?

Yes.

Answer 4

I believe it is a matter of taste, and that you should do both signatures.

However the list of employees should for most beginers -- but it is obviously a matter of what functionality you are trying to implement -- be a list of actual instances, not pointers, and it cannot be references

You should rarely keep a list of pointers, as you likely unless you know what you are doing, will end up in a memory management problem with not freeing or freeing too often -- C++ is not forgiving with errors in memory allocation.

class Company
{
   Company();
   ~Company();

   AddEmployee(const Employee& employee) { employee_.push(employee); }
   AddEmployee(const Employee* employee) { employee_.push(*employee); }
   const Employee& GetEmployee() const;
   Employee& GetEmployee();

private:
   std::list<Employee> employees_;
}

This way you maintain a list of copies of employees inside, and the interface allows somebody who have a pointer as well as an instances to push it by reference.

I general I would avoid handing out pointers to internal data structure, so the GetEmployee should at best hand out a reference (which is also a kind of pointer, but different) -- here you may need to have to accessors -- one for const and one for non const contexts.

Answer 5

This is a list of Employees:

std::list<Employee> employees_;

You could have a list of pointers to Employees, like this

std::list<Employee *> employees_;

but then you'd have to manage the actual Employees (the things the pointers point to) on the heap. Sometimes a good idea, but in this case probably just a pain.

This:

Employee GetEmployee();

returns an Employee, a value, an object, presumably a copy of one of the things in the list. You could have a function like one of these:

Employee & GetEmployee();
Employee * GetEmployee();

which return a reference or pointer (respectively) to an actual Employee. The former is a bad idea in this case because std::list does not promise to keep its contents safe for references; it might make a copy of one of the Employees, keep it and delete the original. The latter has the same problem, but is marginally safer if you go with std::list<Employee *>, because the list will shuffle the pointers but leave the objects alone-- but in that case there's no good way to deal with responsibility for the heap: it makes sense for the Company to construct the Employees, so it's good form for it to delete them also, but then whatever called GetEmployee might wind up with a defunct pointer.

Either of these will do:

AddEmployee(Employee employee);
AddEmployee(Employee & employee);

The second is a little better because it avoids an unnecessary copy. Either way, a copy of the original will wind up in the list. Don't use AddEmployee(Employee * employee); it's perfectly legal but you're just adding opportunity to cause an error by passing it an invalid pointer.