Passing objects to functions safely and efficiently

Question

I would like to have this code reviewed. I am learning C++ from a C background. How would I improve this code to make it more C++ like? This is a simple program that I wrote, which is a stripped down version of the actual full program I'm writing.

#include <iostream>
using namespace std;
class Doge{
public:
 Doge();
 Doge(Doge&);
 ~Doge();
 int GetAge() const { return itsAge; }
 void SetAge(int age) { itsAge = age; }
private:
 int itsAge;
};
Doge::Doge(){
 cout << "Doge Constructor" << endl;
 itsAge = 5;
}
Doge::Doge(Doge&){
 cout << "Doge Copy Constructor" << endl;
}
Doge::~Doge(){
 cout << "Doge Destructor" << endl;
}
Doge * Function( Doge * theDoge){
 cout << "In Function" << endl;
 cout << "Scott is now " << theDoge->GetAge() << endl;
}
int main(){
 cout << "Make Doge" << endl;
 Doge Scott;
 cout << "Scott is " << Scott.GetAge() << endl;
 Scott.SetAge(10);
 Function(&Scott);
 cout << endl;
 return 0;
}

• Quaxton Hale: This is C++ and not C.
• syb0rg: I completely agree with @EngieOP, the C tag is for use on questions that are only written in C. C++ is a superset of C, it is not the same and therefore those tags should rarely be put together.

Answer 1

First off, using namespace std is bad practice. Do not pollute the global namespace. Read this

You are passing objects by pointers which solves the problem of making multiple copies. But you are still using pointers, and that can be cumbersome. And there is the potential danger of the function changing the object.

Instead try passing a const pointer. This protects the object from being called by any non-const method and offers the security of passing by value. **As noted by Janos, you aren't returning anything here? Perhaps you are in your actual program.

const Doge * const Function(const Doge * const theDoge){
 std::cout << "In Function" << std::endl;
 std::cout << "Scott is now " << theDoge->GetAge() << std::endl;
}

Better yet, pass references to the object. This is the preferred way in C++.

const Doge & Function(const Doge & theDoge){
 std::cout << "In Function" << std::endl;
 std::cout << "Scott is now " << theDoge.GetAge() << std::endl;
}

Call function:

Function(Scott);

Using references is preferred because it is safer and easier to use than pointers.

Answer 2

It doesn't feel natural to initialize a Doge with a fixed age:

Doge::Doge(){
 cout << "Doge Constructor" << endl;
 itsAge = 5;
}

It would be more natural to make age a mandatory constructor parameter:

Doge::Doge(int itsAge): itsAge(itsAge) {
 std::cout << "Doge Constructor" << std::endl;
}

---

The copy constructor doesn't actually copy anything. Either remove it, or implement copying, for example:

Doge::Doge(Doge& other){
 std::cout << "Doge Copy Constructor" << std::endl;
 this->itsAge = other.itsAge;
}

---

This function is declared to return a Doge *, but it returns nothing:

Doge * Function( Doge * theDoge){
 cout << "In Function" << endl;
 cout << "Scott is now " << theDoge->GetAge() << endl;
}

This raises a compiler warning, at least with g++.

---

You probably implemented the destructor just to see when it gets called. That's fine, for testing. When you're done, just remember to remove it if it's not needed.

• user54356: (int itsAge): itsAge(itsAge) This won't cause a name collision?
• janos: g++ happily compiles it without warnings
• Quaxton Hale: @DaveSmith This