Click here to Skip to main content
15,896,111 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
#include <iostream>
#define MAX 10

using namespace std;

class Person
{


  private:
    char *name;
    char *surname;
    double age;

  public:

    Person(): name(NULL), surname(NULL), age(0.0L)
    {
        cout << "default costructor activated\n";
    }
    Person(char *n, char *sn, double ag): name(n), surname(sn), age(ag)
    {
        cout << "normal costructor activated\n";
    }
    Person(const Person &p)
    {

        if (name != NULL)
        {
            delete name;
            name = NULL;
        }

        if (surname != NULL)
        {
            delete surname;
            surname = NULL;
        }



        name = new char[strlen(p.name) + 1];
        strcpy(name, p.name);

        surname = new char[strlen(p.surname) + 1];
        strcpy(surname, p.surname);

        age = p.age;

        cout << "copy costructor activated\n";
    }


    ~Person()
    {

        if (name != NULL)
        {
            delete name;
            name = NULL;
        }

        if (surname != NULL)
        {
            delete surname;
            surname = NULL;
        }

        age = 0.0L;
        cout << "destructor costructor activated\n";
    }


    void display(void);

    void setName(char*);
    void setSurname(char*);
    void setAge(double);

    Person *operator = (Person*);

    friend int calculateBirthYear(Person &);
};


Person *Person::operator = (Person *p) //implementation of assignment operator
{

  if (name != NULL)
  {
    delete name;
    name = NULL;
  }

  if (surname != NULL)
  {
    delete surname;
    surname = NULL;
  }



  name = new char[strlen(p->name) + 1];
  strcpy(name, p->name);

  surname = new char[strlen(p->surname) + 1];
  strcpy(surname, p->surname);

  age = p->age;
  cout << "Assigment operator activated\n";

  return this;


}

void Person::setName(char *onoma) //onoma mean name
{
  if (name != NULL)
  {
    delete name;
    name = NULL;
  }

  name = new char[strlen(onoma) + 1];
  strcpy(name, onoma);


}


void Person::setSurname(char *eponumo) //eponumo mean surname
{

  if (surname != NULL)
  {
    delete surname;
    surname = NULL;
  }

  name = new char[strlen(eponumo) + 1];
  strcpy(surname, eponumo);



}


void Person::setAge(double ilikia) //ilikia mean age
{

  age = ilikia;



}


void Person::display()
{

  cout << "name: " << name << "\n";
  cout << "Surname: " << surname << "\n";
  cout << "Age: " << age << "\n";

}



int calculateBirthYear(Person &);

int main(void)
{


  char *onoma = NULL,  *eponumo = NULL;
  double age = 0.0L;
  int BirthYear = 0;
  onoma = new char[50];
  eponumo = new char[50];
  short data = 2, raise = 2;

  Person **p = new Person *[MAX];
  for (int c = 0; c < 10; c++)
    p[c] = NULL;       


  cout << "Enter number of data (max 10, by default 2): ";
  cin >> raise;
  system("cls");

  if (raise > 2 && raise < 11)
    data = raise;

  cout << "\t\t\t Insert Data of " << data << " People \n\n\n";

  for (int i = 0; i < data; i++)
  {

    cout << "\nName: ";
    cin >> onoma;

    cout << "Surname: ";
    cin >> eponumo;

    cout << "Age: ";
    cin >> age;
    cout << "\n\n";

    p[i] = new Person(onoma, eponumo, age);
  }

  system("cls");
  cout << "\t\t\t   People Data\n\n\n";
  for (int i = 0; i < data; i++)
  {

    p[i]->display();
    BirthYear = calculateBirthYear(*p[i]);
    cout << "BirthYear: " << BirthYear;
    cout << "\n\n";
  }


  delete [] p;
  delete [] onoma;
  delete [] eponumo;

  cout << "\n\n\n";
  system("PAUSE");
  return 0;
}


int calculateBirthYear(Person &p)
{

  int currentYear = 2012;
  int birthYear = currentYear - static_cast < int > (p.age);
  return birthYear;


}
Posted

use
std::string name,surname;
your code will be very simple.
 
Share this answer
 
I disagree with one minor point in a response above. For your code:

C++
~Person()
{

    if (name != NULL)
    {
        delete name;
        name = NULL;
    }

    if (surname != NULL)
    {
        delete surname;
        surname = NULL;
    }

    age = 0.0L;
    cout << "destructor costructor activated\n";
}


I am of the opinion that setting the pointers to NULL has value and I would write the code as:

C++
~Person()
{
    delete [] name;
    name = NULL;

    delete [] surname;
    surname = NULL;

    cout << "destructor activated\n";
}


First, if the pointers are NULL, you don't need to check for non-NULL before deleting. This is done by the delete operator. It is legal to "delete NULL;". You are just generating extra code to check for non-NULL. There is also no value in clearing the value of age.

The reason for zeroing is that name and surname are potentially pointers to memory (either stack or heap). Zeroing these after deletion fixes several problems:
(1) If your class is ever instantiated on the stack (it is not in your example but in general could be), the pointers point at deleted memory, which could be incorrectly dereferenced.
(2) If an instantiation of your class experiences a double delete problem, these pointers will not propogate the problem further. [it doesn't fix the problem but prevents further damage if some other code is 'bad'.]
(3) [other stuff; ask Google]

I have written code for a long time and fixed many bugs (my own as well as others) and have adopted a "high indemnity" style of coding that attempts to stop bugs as soon as possible and limit damage when things go awry. This is a big deal to debugging, development and security.

This is also an important enough problem to solve for Microsoft that it is one of the details of the "Security Development Lifecycle (SDL)" and they added a new compiler switch "/sdl" to do this for you in MSVC++11 described here[^].
 
Share this answer
 
v3
Comments
qPCR4vir 2-Jan-13 19:01pm    
Hallo!
-In witch situation can one access person.name after the object person have been deleted?(the reason for name=NULL in the destructor)
-(...and why not nullptr?)
Philippe Mori 2-Jan-13 19:19pm    
As mentionned in SDL article linked above, when the switch /sdl is used, one should not explicitly set the pointer to null... To detect error, an invalid address is much more effective.

Also as mentionned in the article, setting a pointer to null might hide the real problem longer so in the end, it might be harder to find the real bug in the code.

It can happen mainly when there are multiple pointers to the same object of when the pointer is in a scope that has a (much) longer duration than the destroyed object. In the case of a member variable, it might occur if some try to use the deleted object but doing so is never 100% safe as the compiler might used a few bytes inside the object to implement the free object list.

In pratice, it is often preferable to avoid as much as possible explicit memory managment by using std::string, std::vector and std::shared_ptr for example.
H.Brydon 2-Jan-13 23:19pm    
Minor quibble... you say "one should not explicitly set the pointer to null..." which should really be "one does not have to explicitly set the pointer to null..."

There are valid reasons for setting it to either NULL (aka 0 aka nullptr) or a known invalid value. The Microsoft article explores both of these concepts.
Philippe Mori 3-Jan-13 9:36am    
As explained in the article, if you use #sdl option to detect error and your code explicitly set the pointer to null when it should not be necessary (like in a destructor), invalid access that variable will not be detected by the SDL.

If SDL is not used, then hiding the error (set the pointer to null) will typically hide the bug. At first, it will appears that you have stop the bugs but in reality, if you are still using the containing object, your program is still bugged.

You want to explicitly set a pointer to null when the pointer can still be legally used afterwards. It might be for the usual code path or in case of an exception that might occurs before the pointer is set to another valid object.

From a security viewpoint, it might help to try to have an high immunity but even in that case, it might be preferable to uses an inline function to set those pointers as it would be easier to try with an invalid value or not resetting the pointer while testing (for example to find if a bug is dependant on that behaviour) and also to properly document the code and to be able to fully use /sdl option if desired.

In my development, I usually follow the exact opposite direction. Instead of "hiding" problem I would try to expose them by using static checks, assertions and sometime exceptions so that an incorrect program will fails as soon as possible (might also be a compile time or during unit tests).
H.Brydon 3-Jan-13 10:47am    
Paragraph #1: I stand with my statement that "should not" should be "don't have to". Maybe its a French/English thing.

Paragraph #2: zeroing the deleted pointer does not hide the bug. It prevents a bug in process from becoming 3 or 4 or more hard to find bugs. Instead of corrupting the heap in multiple places, you restrict it to corrupting in one place. That is an easier bug to find and diagnose.

Paragraph #3: "...legally used afterwards..." If you are writing a class which contains memory managed objects, leaving a deleted pointer non-NULL gives the opportunity to continue using the deleted memory. This is true for your class code as well as the caller (depends on what your class does). If the memory is not valid, don't point to it.

Paragraph #5: Setting deleted pointer to NULL does not hide the problem. There is no clear way of detecting use of a deleted pointer by static checks, assertions or exception.
Default constructor is called only when you create an object using parameter-less constructor...

I have found a few mistakes or possible improvements in your code:

C++
#define MAX 10

It would be preferable to uses a constant instead. Macros should be avoided whenever possible as they don't respect scope.

C++
Person(const Person &p)
{
    if (name != NULL)
    {
        delete name;
        name = NULL;
    }

    if (surname != NULL)
    {
        delete surname;
        surname = NULL;
    }
    // ...

This code is totally useless and in fact wrong. Since copy constructor is called on a new object which is uninitialized, you cannot delete name or surname.
C++
~Person()
{
    if (name != NULL)
    {
        delete name;
        name = NULL;
    }

    if (surname != NULL)
    {
        delete surname;
        surname = NULL;
    }
    age = 0.0L;

    cout << "destructor costructor activated\n";
}

Most of previous code is essentially useless. When an object is destroyed, it cannot be legally used afterward so it pointless to clear all variables. You could simply do that instead:
C++
~Person()
{
    delete[] name;
    delete[] surname;

    cout << "destructor costructor activated\n";
}


C++
void setName(char*);
void setSurname(char*);

Person *operator = (Person*);

friend int calculateBirthYear(Person &);

In previous function you should add some const modifiers and also, you should use standard prototype for =operator. It should be Person& operator=(const Person &);
Also calculateBirthYear should not be friend. You should have a public member to get the age. By the way, the function cannot be implemented correctly because you need to know the current date and the birthday to properly determine the age.
C++
Person *Person::operator = (Person *p) //implementation of assignment operator
{
    // ...
}

Your implementation is not exception safe if memory cannot be allocated... So are some other functions. Generally, you want to do new allocation before deleting old stuff so that if it does not works, your object will still be in a stable state. Using RAII idiom is a good pattern to follows.
C++
name = new char[strlen(onoma) + 1];
strcpy(name, onoma);

This code is repetitive. Why not add a static member function for that purpose?
C++
int main(void)
{
  //...
}

The main function has some memory leaks (Person in the array are not deleted).

Also, there are still a bunch of hard-coded constants. For example if MAX is modified then neither the message nor the range check are adjusted.

As a side note, it would be better to use std::vector and std::string in your program to avoid a lot of explicit memory management and have code that it is exception-safe.
 
Share this answer
 
v2
Comments
qPCR4vir 2-Jan-13 18:48pm    
Could you comment delete name; vs delete []name; ?
Philippe Mori 3-Jan-13 9:57am    
If the allocation was made with array new operator, then the deletion should uses array deletion. That is since name = new char[some_size]; is used for allocation, then delete[] name; must be used for deletion. The fact that it might seems to properly works does not justify you from doing things that are illegal... It is not guaranteed to works in all cases and array of objects where the class have a destructor, it will fails to call the destructor for all objects (except the first). When an array of memory is allocated, the compiler must keep information about the number of elements that were allocated. This is not necessary for simple allocation as the object type (or its virtual table) can be used to know the actual size. All this is implementation dependant and thus one should not assumes that they can be used interchangeably.
Yes. default constructor is not called because you havent written any code to call it.
By executing this line
C++
p[i] = new Person(onoma, eponumo, age);
it finds the matching constructor which has parameters defined and gets called and not the default or parameter none constructor.

Yes copy constructor is not called because you havent written any code to call it.
copy constructor gets call called in following three occasions
1.When instantiating one object and initializing it with values from another object (as in the example above).
2.When passing an object by value.
3.When an object is returned from a function by value.

ie
C++
main
{
 Person p(on0ma,epunumo,age);
 Person p1 = p; // Copy Constructor gets called
}

Read more about Copy Constructor @CPlusPlus Forum[^]
 
Share this answer
 

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900