Cpp some basic problems

Question

My task was as follows : Create class Person with char*name and int age. Implement contructor using dynamic allocation of memory for variables, destructor, function init and friend function show. Then transform this class to header and cpp file and implement in other program. Ok so here's my Person class :

#include <iostream>
using namespace std;

class Person {   
    char* name;
    int age;
public:   

    Person(){
        int size=0;
        cout << "Give length of char*" << endl;
        cin >> size;
        name = new char[size];      
        age = 0;
    }

    Person::~Person(){
        cout << "Destroying resources" << endl;
        delete [] name;
        delete take_age();
    }  

    friend void show(Person &p);

   int* take_age(){
       return &age;
   }

   char* take_name(){
         return name;      
   }

    void init(char* n, int a) {
        name = n;
        age = a;
    }
}; 

void show(Person *p){
    cout << "Name: " << p->take_name() << "," << "age: " << p->take_age() << endl; 
}

int main(void) {
    Person *p = new Person;  
    p->init("Mary", 25);

    show(p);

    system("PAUSE");
    return 0;
}

And now with header/implementation part :
- do I need to introduce constructor in header/implementation files ? If yes - how?
- my show() function is a friendly function. Should I take it into account somehow ?

I already failed to return this task on my exam, but still I'd like to know how to implement it.

Answer 1

Solve many of your issues, by switching from char * to std::string. You'll be glad you did.

The std::string class takes care of memory allocation, and deallocation as well as copying.

If this is homework, convince your professor to use std::string for beginners and save char * for the section on pointers. Also remind your professor that the C++ langauge is different than the C language. This is one of those areas.

Answer 2

You don't need a * when using delete or delete[]. Just supply a pointer variable to it eg.

delete[] name;

Also, your take_age member claims to return a int* but you actually return the int member itself. You need to take the address of the member using & if you want to do that. As @Jerry has commented this is not what you want to do here.

Answer 3

I think you should investigate the following pieces of your code (like, what's beneath them, what happens here, etc...)

int * take_age(); // You should return plain `int` here, I assume


~Person(){
    cout << "Destroying resources" << endl;
    delete *[] name; // Do you understand why did you put `*` here?
    delete * take_age(); // Do you understand why did you write this? What behaviour you were trying to achieve?

And, actually, so on. Only when you're done with the basic stuff, I think, you can move on to header designing questions and friend functions.

Answer 4

Although some on this site apparently think it is completely acceptable, good practice (see Can a constructor return a NULL value?), you should really refrain from doing things like stream operations within the constructor of your object. Do that stream reading outside and then call the function with the results.

That is, IMHO, the first step you should take.

Answer 5

I think your problem is with this line: friend void(Person &p); What is it needed for.

do I need to introduce constructor in header/implementation files ? The constructor can be in the .h or the .cpp file. It doesn't matter. Generally if the function is short it is ok to include it in the .h file. Anything longer should go in the .cpp.

my show() function is a friendly function. Not sure what you mean by this. friend functions exist outside the class definition. Your show function is defined inside the class so does not need to be a friend.

Answer 6

In a typical case, managing a pointer and block of dynamically allocated memory (such as the name in this case) is enough responsibility for one class. As such, Thomas Matthews is right: you should really use string in this case. If you're going to handle it yourself, you should still split that responsibility off into a class of its own, and embed an object of that class into your Person object. If anything, std::string already tries to do too much; you'd be better off with something that does less, not more.

Your deletes should exact match with your allocations. In this case, the only allocation is:

    name = new char[size];      

so the only deletion should be:

    delete [] name;

As far as friend functions go, you normally want the friend declaration inside the class definition, but the function definition outside the class definition:

class Person { 
// ...
    friend void show(Person const &);
// ...
};

void show(Person const &p) { 
     // ...
}

There are other possibilities, but that's the general idea. In particular, a friend is never a member function. What you had was a declaration of one (global) function named show and a definition of a completely separate member function -- that happened to have the same name, but wasn't really the same function at all.

That shows one other point: const-correctness. You were passing the parameter as a reference to Person. Unless it's going to modify the Person object (in which case, show() seems like a poor choice of name), it should probably take a reference to a const object. The same general idea applies to take_age() -- since it only retrieves a value, it should be a const function:

int take_age() const { return age; }

I've probably already tried to cover too much, so I'll shut up for the moment...

Answer 7

First off, kudos on trying to find the right way to implement your class, particularly after having missed the answer already.

From your description at the top, I think you may have misunderstood some of what was being asked for this assignment. First, my interpretation would be that setting the value of the name and age should take place in the init() function rather than in the constructor. As mentioned by several other posters, your constructor should simply initialize your class to a known-good state. For example,

Person() {
    name = NULL;
    age = 0;
}

Then in your initialization function, you can assign the values. Looking at your original init() function, it should probably be mentioned that simply assigning a pointer value (char *) to another pointer (char *) only copies the value of the pointer, not the data that it represents. Thus, for the assignment of the name value you need to calculate the size of the buffer you need, allocate the buffer, and copy the data yourself. A basic init() function would probably look like

init(const char *n, int a) {
    // Calculate the required name length plus a NULL-terminator
    size_t nameLen = strlen(n) + 1;

    // Free any previous value.
    if (name != NULL) {
        delete[] name;
    }

    // Allocate the name buffer and copy the name into it.
    name = new char[nameLen];
    memcpy(name, n, nameLen);

    // Store the age.
    age = a;
}

Finally, in your destructor you free any resources allocated by your class, in this case the name buffer.

~Person() {
    if (name != NULL) {
        delete[] name;
    }
}

If you have a book or something associated with your class, you may want to review the information on pointers. They can be a bit tricky but are important to learn. I suspect that is why the problem specified using char * for strings rather than the STL string class.

To your question about placing information in header and source files, it is often considered good practice to create a header file that contains the class declaration and member function prototypes and then provide the implementation of your methods in a separate source file. For some simple functions, you can provide an implementation directly in your header file.

The key when providing class member definitions in a separate source file is to provide the class name to properly scope the function (i.e., Person::). So your header file may contain a class definition like

// Header file (e.g., person.h)

class Person {
private:
    char *name;
    int age;

public:
    Person() { name = NULL; age = 0 };
    ~Person() { if (name != NULL) delete[] name; }

    void init(const char *n, int a);

    // Other method declarations and/or definitions
};

And then in your source file

// Source file (e.g., person.cpp)

void Person::init(const char *n, int a) {
    // ...
}

// Rest of method definitions

Source files that use your person class need only include the header file with your class definition.

Answer 8

in addition to the previously posted answers, i've got two points of advice for you:

• don't use 'friend'. some here may disagree with me, but 'friend' should really not be part of C++ anymore as it goes against what OOP stands for.
• naming your methods: avoid naming your methods like 'take_name' or 'take_age'. conventionally, since those are getters, consider naming them 'getName' and 'getAge'. you end up with much more respect from developers this way.