Does class inherit correctly class "Person"

[theateist]

I need to create 4 classes:
1 - Describe a person
2 - Describe a director
3 - Describe a actor
4 - Describe a person who is a director and actor

Did I created them correctly?
Is it wrong that classes DirActor, Actor and Director inherit class "Person"? Or I should do it in another way?
Please do not correct me about using "char*" instead of "string" because I was asked to use "char*".

Thank you very much.

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

const int MAX_LEN_OF_NAME = 100;

class Person
{
	char* m_fname;
	char* m_lname;
	int m_age;
public:
	Person(): m_fname(NULL), m_lname(NULL){}
	~Person()
	{
		delete[] m_fname;
		delete[] m_lname;
		m_fname = m_lname = NULL;
	}
	Person(const char* strFirstName, const char* strLastname, int iAge) : m_age(iAge), m_fname(NULL), m_lname(NULL)
	{
		SetName(strFirstName, strLastname);
	}
	//Set name of the person
	void SetName(const char* strFirstName, const char* strLastname)
	{
		//Don't reallocate memory if the length of the new name is less than the current one
		if(m_fname == NULL || strlen(m_fname) < strlen(strFirstName))        
		{
			delete[] m_fname;
			m_fname = new char[strlen(strFirstName) + 1];
		}
		strcpy(m_fname, strFirstName);        
		if(m_lname == NULL || strlen(m_lname) < strlen(strLastname))        
		{
			delete[] m_lname;
			m_lname = new char[strlen(strLastname) + 1];
		}
		strcpy(m_lname, strLastname);
	}
	void SetAge(int iAge)
	{
		m_age = iAge;
	}
	const char* GetFirstName() const
	{
		return m_fname;
	}
	const char* GetLastName() const
	{
		return m_lname;
	}
	int GetAge() const
	{
		return m_age;
	}
};
class Movie
{
	char* m_name;
public:    
	Movie() : m_name(NULL)
	{        
	}
	~Movie()
	{        
		delete[] m_name;
		m_name = NULL;
	}
	void SetName(const char* strName)
	{
		if(m_name == NULL || strlen(m_name) < strlen(strName))
		{
			delete[] m_name;
			m_name = new char[strlen(strName) + 1];
		}
		strcpy(m_name, strName);
	}
	const char* GetName() const
	{
		return m_name;
	}
};
class Director : public Person
{
	Movie* m_movies;
	int m_numOfMovies;
public:    
	Director(const char* strFirstName, const char* strLastname, int iAge) : m_numOfMovies(0), m_movies(NULL)
	{
		//Set name of the director
		SetName(strFirstName, strLastname);
		SetAge(iAge);            
	}
	~Director()
	{
		delete[] m_movies;
		m_movies = NULL;
	}
	//Allocate memory for specific number of movies
	void SetNumOfMovies(int iNum)
	{
		if(m_numOfMovies == iNum)  
			return;

		m_numOfMovies = iNum;

		if(m_numOfMovies > 0)
		{
			delete[] m_movies;
			m_movies = new Movie[m_numOfMovies];
		}
	}    
	void SetMovies()
	{
		int iNumOfMovies;        

		cout << "How many movies he produced?" << endl;
		cin >> iNumOfMovies;

		SetNumOfMovies(iNumOfMovies);

		char movieName[MAX_LEN_OF_NAME];
		for(int i = 0; i < m_numOfMovies; i++)
		{
			cout << "What the name of the " << i + 1 << " movie?" << endl;
			cin >> movieName;
			m_movies[i].SetName(movieName);
		}
	}
	const Movie* GetMovies() const
	{
		return m_movies;
	}
	int GetNumOfMovies() const
	{
		return m_numOfMovies;
	}
};
class Actor : public Person
{
	Movie* m_movies;
	int m_numOfMovies;
public:    
	Actor(const char* strFirstName, const char* strLastname, int iAge) : m_numOfMovies(0), m_movies(NULL)
	{
		//Set name of the director
		SetName(strFirstName, strLastname);
		SetAge(iAge);                
	}
	~Actor()
	{
		delete[] m_movies;
		m_movies = NULL;
	}
	//Allocate memory for specific number of movies
	void SetNumOfMovies(int iNum)
	{
		if(m_numOfMovies == iNum)  
			return;

		m_numOfMovies = iNum;

		if(m_numOfMovies > 0)
		{
			delete[] m_movies;
			m_movies = new Movie[m_numOfMovies];
		}
	}    
	void SetMovies()
	{
		int iNumOfMovies;        

		cout << "In how many movies he played?" << endl;
		cin >> iNumOfMovies;

		SetNumOfMovies(iNumOfMovies);

		char movieName[MAX_LEN_OF_NAME];
		for(int i = 0; i < m_numOfMovies; i++)
		{
			cout << "What the name of the " << i + 1 << " movie" << endl;
			cin >> movieName;
			m_movies[i].SetName(movieName);
		}
	}
	const Movie* GetMovies() const
	{
		return m_movies;
	}
	int GetNumOfMovies() const
	{
		return m_numOfMovies;
	}
};
//Class for person who is Director and Actor
class DirActor : public Person
{
	Director* m_director;
	Actor* m_actor;
public:    
	DirActor(const char* strFirstName, const char* strLastname, int iAge)
	{        
		SetName(strFirstName, strLastname);
		SetAge(iAge);
		m_director = new Director(strFirstName, strLastname,iAge);
		m_actor = new Actor(strFirstName, strLastname,iAge);
	}
	~DirActor()
	{
		delete m_director;
		m_director = NULL;
		delete m_actor;
		m_actor = NULL;
	}
	Director* GetDirector() const
	{
		return m_director;
	}
	Actor* GetActor() const
	{
		return m_actor;
	}
	void PrintDetails()
	{
		cout << "=============================================================================" << endl;
		cout << "Printing information about " << GetFirstName() << " " << GetLastName() << endl;
		cout << "=============================================================================" << endl;
		cout << "Age:" << GetAge() << endl << endl;
		cout << "The movies he produced are:" << endl;
		for(int i = 0; i < m_director->GetNumOfMovies(); i++)
		{    
			cout << i + 1 << " movie:" << m_director->GetMovies()[i].GetName() << endl;
		}
		cout << endl;
		cout << "The movies he played in are:" << endl;
		for(int i = 0; i < m_actor->GetNumOfMovies(); i++)
		{    
			cout << i + 1 << " movie:" << m_actor->GetMovies()[i].GetName() << endl;
		}
		
	}	
};
void main()
{    
	DirActor da("Clint", "Eastwood", 79);
	da.GetDirector()->SetMovies();
	da.GetActor()->SetMovies();  
	da.PrintDetails();
}

[guestgulkan]

I would say if you are using c++ (which you are ) don't bother with char* for the names - use string in stead. This will clean up the setname function of the person class
and incidentally it will also straighten out most of the destructors which currently have the potential to generate segmentation faults.

[theateist]

Thank you for you answer, but in school we were asked to use "char*". In my opinion there won't segmentation faults becuase I put NULL to pointer after deleting.

Beside this, is my inheritence and declaration of the DirActor is correct?

Thank you

[jfq722]

Hi, I'll get to your inheritance question, but firstly, what you do with the Person constructor (deallocating and reallocating name memory) - is only needed on COPY CONSTRUCTORS and ASSIGNMENT OPERATORS, not on regular constructors. You say:

Person(const char* strFirstName, const char* strLastname, int iAge) : m_age(iAge), m_fname(NULL), m_lname(NULL)
{
SetName(strFirstName, strLastname);
}

For example, with the above constructor you are allocating a brand new object, so the concept of "old one and new one" as you say in your comments, does not apply. There is only the new one. I understand that you can't set the name variables on the base initializer list since they are char *. The fact that SetNames() deallocates and reallocates memory is great! - but the deallocation is needed on the COPY CONSTRUCTORS and ASSIGNMENT OPERATORS (where the concept of old and new objects really applies) - and in fact the conditionals may be more trouble than they are worth. I would just deallocate and reallocate regardless of size - but thats my opinion ;)

I would also make sure that SetAge() & SetNames() are defined as private functions so that only the Person functions can use them. You wouldn't want a theateist person object created with the age of 25, then later on someone sets the age to 40 because they feel like it ;)) Although if you want to change my age to 25 from 47, go right ahead. Otherwise, your Person class looks good to me.

Movie class also looks ok - but the same comments I made about Person apply to Movie.

To have Director derive publicly from Person is fine. But here is how the Director constructor should look. Remember, ideally, Person's SetNames() & SetAge() will be private and unavailable to anyone including the Director class. You say:

Director(const char* strFirstName, const char* strLastname, int iAge) : m_numOfMovies(0), m_movies(NULL)
{
//Set name of the director
SetName(strFirstName, strLastname);
SetAge(iAge);
}

BUT YOU SHOULD CHANGE THIS TO:

Director(const char* strFirstName, const char* strLastname, int iAge) : Person(strFirstName, strLastname, iAge), m_numOfMovies(0), m_movies(NULL)
{
//Set name of the director

No need to invoke SetName or SetAge as the already tested
Person constructor will do that. This is where the work you've
done on the Person class begins to pay off;
}

Also, ideally, by the time an object is constructed it should be fully ready for use. Its not a great idea to leave it up to the user to finish building it manually. For example after I instantiate a Director object, I'll have to remember to set his number of movies. And if I don't, I won't have them! It's proabably better to have the number of movies passed as the final argument into the Director constructor.

Actor deriving from Person is ok, but the same notes apply.

However, DirActor is where your design has a real problem. Here is what you say:

class DirActor : public Person
{
Director* m_director;
Actor* m_actor;
}

This basically says that a DirActor IS A PERSON WHO HAS/USES A DIRECTOR AND AN ACTOR.

It is probably best to use MULTIPE INHERITANCE here. I know that is frowned upon, but DirActor is alot like SleeperSofa - there aren't many other ways to do it. Here is what you should say:

class DirActor : virtual public Director, virtual Public Actor
{
You may run into the "diamond" problem. If your DirActor class tries to invoke
a base class member function that is defined in BOTH Director and Actor.
I believe the 'virtual' above will avoid this by causuing C++ to only include 1
version of a mutiply defined base class. And remember, on DirActor's base
initializer list, you should initialize Director and Actor the same way you initialized
Person on your Director's base initializer list! This may mean giving DirActor ALOT of
arguments to his constructor, but that is a small price to ask in exchange for all of
the fully tested Director and Actor functionality you will get; not to mention the
ability to combine them in this way.
}

Hope this helps!

[jfq722]

Oh, and lastly, don't forget to make your Person destructor virtual.
Anytime a class is going to be used as a base class or anytime a class has at least 1 virtual function, the destructor of that class should be defined as virtual.

This guarantees that ALL parts of the base/derived class sandwhich are fully destructed when the time comes.

So if all the changes we talked about are made, your main section might look like this:

void main()
{
DirActor da("Clint", "Eastwood", 79, also any other args that might be needed);
da.PrintDetails();
}

[theateist]

Thank you very much for yout help.
I really appreciate this.:)))

But still, I have 1 more question. After all changes, compiler says that there is am ambiguous access of GetFirstName, GetLastName, GetAge in function PrintDetails().
Shoul I change it to Person::GetFirstName(), Person::GetLastName(), Person::GetAge ()?

Thank you again.

[jfq722]

Can you repost the code and the error message? or send to [e-mail] along with the error message which may be easier?

[DaveMortimer]

Here is a minimal statement of requirements for your problem...

A person may play zero, one or more roles (such as director, actor, etc.). A particular role is played by zero, one or more persons.
A movie has one or more roles which need to be fulfilled. A particular role may be played in zero, one or more movies.

The following diagram represents a structural model for the above requirements. The orchestrating instance (OI) is movieAdmin. Basically, the OI takes input from the user interface (UI) domain and returns output to the UI. The OI is the only instance of the orchestrating class MovieAdmin which tracks objects in the model domain. The objects which the OI tracks depends on the use cases (features) that you include in the application...

[movieAdmin]1---persons---1..*[Person]
[Person]0..*---plays---0..*[Role]
[Role]1..*---isPlayedIn---0..*[Movie]

(Join these up into a structural model diagram.)
-------------------------------------------
class MovieAdmin
  association instance variables
    Persons ---> aDictionary( aString ---> aDictionary( aString ---> aPerson ... ) ... )
  use case handlers
    AllMovies( inFirstName : String, inLastName : String ) : SetOfString
    DirectedAndFeaturedIn( inFirstName : String, inLastName : String ) : SetOfString
-------------------------------------------
class Person
  attribute instance variables
    FirstName ---> aString
    LastName ---> aString
    DateOfBirth ---> aString
  association instance variables
    Roles ---> aSet( aRole ... )
-------------------------------------------
class Role
  attribute instance variables
    RoleName ---> aString
  association instance variables
    Movies ---> aSet( aMovie ... )
-------------------------------------------
class Movie
  attribute instance variables
    MovieName ---> aString
-------------------------------------------

This is sometimes called 'responsibility driven design'. For example, from the use cases below, we have stipulated that a person is responsible for keeping track of the roles that he or she plays, and that a role is responsible for keeping track of the movies that require it. Further use cases will assign further responsibilities to the classes.

Also, there are two very important concepts which must be understood within the current context - 'coupling' and 'cohesion'. Try to practise your design with these in mind.

Here are a couple of use cases for the model...

*******************************************************************
Use case 0001: AllMovies( inFirstName : String, inLastName : String )

Given: A first name inFirstName. A last name inLastName.

Result: Return a set containing the movie names which the given person has directed, featured in, or both directed and featured in.

(1)   Locate the instance aPerson, of Person, with first name inFirstName and last name inLastName, linked to movieAdmin via persons.

(2)   Locate all the instances of Role linked to aPerson via plays.

(3)   For each instance aRole located in step (2)...
      {
(4)      Locate all the instances of Movie linked to aRole via isPlayedIn.
      }
(5)   Form a set aSet( aMovie ... ) from the Movie instances located in step (4).

*******************************************************************
Use case 0002: DirectedAndFeaturedIn( inFirstName : String, inLastName : String )

Given: A first name inFirstName. A last name inLastName.

Result: Return a set containing the movie names which the given person has both directed and featured in.

(1)   Locate the instance aPerson, of Person, with first name inFirstName and last name inLastName, linked to movieAdmin via persons.

(2)   Locate all the instances of Role linked to aPerson via plays.

(3)   Declare a set aSetOfMovie, of SetOfMovie, to contain Movie instances.

      For each instance aRole located in step (2)...
      {
         If aRole.RoleName == 'Director', OR, aRole.RoleName == 'Actor'...
         {
(4)         Locate all the instances of Movie linked to aRole via isPlayedIn.

(5)         Insert into the set aSetOfMovie, all the Movie instances located in step (4).
         }
      }

// aSetOfMovie is the set of movies which the given person has both directed and featured in.

*******************************************************************

Now you try. For example, write a use case for finding which movies a given person acted in, and add the association responsibilities to the class specifications above. (This may take you a couple of weeks if you have never done this before. Buy a good book or enrol on a course, you'll never look back.)

I understand that all this may sound complicated. If you have never done this before, then I'm hitting you with quite a lot here. But if you are willing to spend a few months on the study of object theory, then you will never again have to ask such basic questions, and you will be able to see that implementation tools such as C++ are not design tools.

By the way, notice that the class methods have yet to be established. As with the association responsibilities, behavioural responsibilities can be extracted from the use case walk-throughs along with another type of diagram which shows how the message-passing between objects is organised.

Dave