Yet another Memory Leak Question

fp_unit · 02-09-2005, 08:19 PM

I'm trying to help my friend with an assignment that he has absolutely no clue on. He confided in me that programming isn't what he wants to do, and he's switching his major next year. He wants to keep up good grades so he can switch to engineering easily, normally I wouldn't practically write someone's assignment for them, but I understand his situation so I'm trying to help him out (btw he doesn't go to my school).

He's in Intro to OOP using C++, and his assignment is to develop a Video Rental System for a shop that rents out DVD's and VCD's (Video CD's) to it's customers. The system must keep track of its rental information and compute rental fees from the DVD's and VCD's rented out to customers.

Objects are required for this assignment. At minimum, a class for a Customer, and a class for a Movie. My friend implemented these. I also implemented a VideoStore class to manage objects of type Customer and Movie. I'm trying to keep it as simple as possible for him, after all, he has to explain to his teacher what he did. I'm trying to set up a skeleton and a few methods so he can do the rest. The original code wasn't very pretty. The leak seems to be in the class I wrote (VideoStore) in method addMovie(). Other methods in the class definition haven't been written yet.

The program compiles and executes, but look at function main()

Code:

#include "videostore.h"

int main()
{
	VideoStore v1;
	v1.addMovie();
	v1.addMovie();
	v1.printMovies();

	return 0;
}

addMovie prompts for 2 char array strings, then is supposed to copy them to their respective object (Movie). The program crashes and exits after the second string is entered, and v1.addMovie(); never executes a second time.

Here's the code:

Code:

customer.h

#ifndef CUSTOMER_H
#define CUSTOMER_H

class Customer {

	char Name[40];
	int Age;
	char CustCode[10];

public:
	static int cCount;

	Customer()
	{
		Name[0]     = '\0';
		CustCode[0] = '\0';
		Age = 0;
	}


	Customer(char *n, char *c, int a)
	{
		strcpy(Name,n);
		strcpy(CustCode, c);
		Age = a; 
	}

	void setCustomer(char *n, char *c, int a)
	{
		strcpy(Name,n); 
		strcpy(CustCode, c);
		Age = a; 
	}

	void display()
	{
		cout << Name << "  " << Age << " " << CustCode << endl;
	}

	char *theName()
	{
		return Name;
	}

	int theAge() 

	{
		return Age;
	}
};

#endif

Code:

movie.h


#ifndef MOVIE_H
#define MOVIE_H

class Movie
{
private:

	char title[40];
	char desc[5];
	int serialnum;

public:
	static int mCount;

	Movie()
	{
		title[0] = '\0';
		desc[0]  = '\0';
		serialnum = 0;
	}

	Movie(char *t,char *d)
	{
		strcpy(title, t);
		strcpy(desc, d);
	}

	char* getTitle()
	{
		return title;
	}

	void setMovie(char* t, char* d)
	{
		strcpy(title, t); 
		strcpy(desc, d);
	}

	void displayMovie()
	{
		cout << title << "  " << desc << endl;
	} 

};

#endif

Code:

videostore.h


#ifndef VIDEOSTORE_H
#define VIDEOSTORE_H


#include "movie.h"
#include "customer.h"

class VideoStore
{
private:

	Customer* customerArrayPtr;
	Movie*    movieArrayPtr;

	unsigned numMovies;
	unsigned numCustomers;

public:

	VideoStore();
	~VideoStore();

	void addMovie();
	void modifyMovie();
	void printMovies();

	void addCustomer();
	void modifyCustomer();
	void deleteCustomer();

};


VideoStore::VideoStore()
{
	customerArrayPtr = 0;
	movieArrayPtr    = 0;

	numMovies    = 0;
	numCustomers = 0;

}


VideoStore::~VideoStore()
{
	delete [] customerArrayPtr;
	delete [] movieArrayPtr;
}


void VideoStore::addMovie()
{
	numMovies++;

	Movie * tmpPtr = new Movie[ numMovies];
	char tTitle[40];
	char tDesc[5];
	
	// copy array, element by element
	if(numMovies != 1)
	{
		for(int i = 0; i <= numMovies; i++)
			tmpPtr[i] = movieArrayPtr[i];
			delete [] movieArrayPtr;
			// just have movieArrayPtr point to the new array
			movieArrayPtr = tmpPtr;
			delete [] tmpPtr;
	}

    // are you gonna actually add a movie in here somewhere
    // like the function name suggests?  You probably want to take 
    // a Movie object as a paramter. better yet a reference to a Movie
    // object, make sure your Movie class has a copy constructor 
    // and operator= function

	// do input/output to read movie into VideoStore
	cout << "Enter Movie Name: ";
	cin.getline(tTitle, 40);

	cout << "Enter DVD or VCD: ";
	cin.getline(tDesc, 5);

	movieArrayPtr[numMovies].setMovie(tTitle, tDesc);

}


void VideoStore::modifyMovie()
{
	char tTitle[40];
	char tDesc[5];
	int i;
	bool isFound = false;

	cout << endl;
	cout << "Enter Movie Name Which You Wish To Modify : ";
	cin.getline(tTitle,40);

	for(i = 0; i < numMovies; i++)
	{ 
		if ( stricmp(tTitle, movieArrayPtr[i].getTitle()) == 0 )
		{
			cout << "\nRe-Enter Movie Name : ";
			cin.getline(tTitle, 40);
			cout << "\nRe-Enter Medium Type, e.g VCD, DVD : ";
			cin.getline(tDesc, 5);
			movieArrayPtr[i].setMovie(tTitle, tDesc);
			break;
		}

	}
	
	if(!isFound)
		cout << "Name Not In The List!" << endl;
}


void VideoStore::printMovies()
{
	for(int i = 0; i < numMovies; i++)
		movieArrayPtr[i].displayMovie();
}


#endif

And here's main

Code:

main.cpp


#include <iostream>
using namespace std;

//#include "customer.h"
//#include "movie.h"
#include "videostore.h"

int main()
{
	VideoStore v1;
	v1.addMovie();
	v1.addMovie();
	v1.printMovies();

	return 0;
}

Help would be very appreciated.

fp_unit · 02-09-2005, 08:43 PM

Fixed that function, alls OK now. Still, the classes are sloppy if anyone wants to improve their design an/or relationships, feel free!

Code:

void VideoStore::addMovie()
{
	numMovies++;

	movieArrayPtr = new Movie[numMovies];
	char tTitle[40];
	char tDesc[5];
	
	if(numMovies != 1)
	{
		Movie * tmpPtr = new Movie[numMovies + 1];
		cout << "In if case" << endl;
	
		// copy array, element by element
		for(int i = 0; i <= numMovies; i++)
			tmpPtr[i] = movieArrayPtr[i];
		
		delete [] movieArrayPtr;
		movieArrayPtr = tmpPtr;
	}

	cout << "Enter Movie Name: ";
	cin.getline(tTitle, 40);

	cout << "Enter DVD or VCD: ";
	cin.getline(tDesc, 5);

	// Add movie to correct location
	movieArrayPtr[numMovies - 1].setMovie(tTitle, tDesc);

}

Valmont · 02-09-2005, 11:31 PM

Here is my code using C++ only. You are mixing C with C++.
- You need to add a copy constructor.
- You need to add an overloaded operator=().
- You need to implement serial number for the movie class and the manager class.

Code:

#ifndef CUSTOMER_H
#define CUSTOMER_H

#include <string>
#include <iostream>

using std::ostream;

class Customer
{
public:
  Customer() :
   sName_(""), sCustCode_(""), Age_(0)
   {   }
   Customer(std::string n, std::string c, unsigned a) :
   sName_(n), sCustCode_(c), Age_(a)
   {   }
   //
   void setCustomer(std::string n, std::string c, int a)
   {
     sName_ = n;
     sCustCode_ =  c;
     Age_ = a;
   }
   // Too lazy to make the proper accessors.
   std::string& name()
   {
     return sName_;
   }
   std::string& code()
   {
     return sCustCode_;
   }
   unsigned& age()
   {
     return Age_;
   }
private:
  unsigned Age_;
  std::string sName_;
  std::string sCustCode_;
  static int cCount;
  friend ostream& operator<<(ostream& os, const Customer& cst);
};

ostream& operator<<(ostream& os, const Customer& cst)
{
  os << cst.sName_ << " " << cst.Age_ << " " << cst.sCustCode_;
  return os;
}


#endif //CUSTOMER_H

Code:

#ifndef MOVIE_H
#define MOVIE_H

#include <string>
#include <iostream>

using std::ostream;

class Movie
{
public:
  static int mCount;
  Movie() :
    sTitle_(""), sDesc_(""), serialNum_(0)
    {  }
  Movie(std::string t, std::string d, int s) :
    sTitle_(t), sDesc_(d), serialNum_(s)
    {  }
    //
    std::string getTitle() const
    {
      return sTitle_;
    }
    //
    void setMovie(std::string t, std::string d)
    {
      sTitle_ = t;
      sDesc_ = d;
    }
private:
  std::string sTitle_;
  std::string sDesc_;
  int serialNum_;
  friend ostream& operator<<(ostream& os, const Movie& mvy);
};

ostream& operator<<(ostream& os, const Movie& mvy)
{
  os << mvy.sTitle_ << " " << mvy.sDesc_ << " " << mvy.serialNum_;
  return os;
}

#endif //MOVIE_H

Code:

#ifndef RENTALMANAGER_H
#define RENTALMANAGER_H

#include "Customer.h"
#include "Movie.h"
#include <string>
#include <iostream>
#include <climits>

using std::string;
using std::cout;
using std::cin;
using std::endl;
using std::getline;

class RentalManager
{
public:
  RentalManager() : rc_(0), rcnSize_(0)
  { }
  ~RentalManager()
  {
    delete[] rc_;
  }
  //
  void add_new_record()
  {
    resize_record_with_one();
    
    cout<<"Customer Code: ";
    string c;
    getline(cin, c);
    cout<<"Title: ";
    string t;
    getline(cin, t);
    cout<<"DVD (d) or VCD (v): ";
    string d;
    cin>>d;
    cin.ignore(INT_MAX, '\n');
    
    //Does customer code (and therefore, is he a member) exist?
    unsigned i = 0;
    for(; i < rcnSize_; ++i)
    {
      if(rc_[i].Customer_.code() == c)
      {
        break; //It does exist.
      }
    }

    if(i ==  rcnSize_) //Customer Code does not exist.
    {
      string n;
      unsigned a;
      cout<<"NOTIFICATION: Customer is not a member."<<endl;
      cout<<"Customer Name: ";
      getline(cin, n);
      cout<<"Customer Age: ";
      cin>>a;
      cin.ignore(INT_MAX, '\n');
      rc_[rcnSize_-1].Customer_.setCustomer(n, c, a);
    }
    else //Customer Code exists.
    {
      rc_[rcnSize_-1].Customer_.code() = c;
      //Customer is member already so let's find his name/age by Code.
      for(unsigned i = 0; i < rcnSize_; ++i)
      {
        if(rc_[i].Customer_.code() == c)
        {
          rc_[rcnSize_-1].Customer_.name() = rc_[i].Customer_.name();
          rc_[rcnSize_-1].Customer_.age() = rc_[i].Customer_.age();
          break;
        }
      }
    }
    rc_[rcnSize_-1].Movie_.setMovie(t, d);
  }
  void view_all_actions() const
  {
    for(unsigned i = 0; i < rcnSize_; ++i)
    {
      cout<<"Code: "<<rc_[i].Customer_.code()<<endl;
      cout<<"Name: "<<rc_[i].Customer_.name()<<endl;
      cout<<"Age: "<<rc_[i].Customer_.age()<<endl;
      cout<<"Movie Data (title, descr., serial #): "<<rc_[i].Movie_<<endl<<endl;
    }
  }
  //
private:
  struct Record
  {
    Customer Customer_;
    Movie    Movie_;
  };
  Record* rc_;
  unsigned rcnSize_;
  void resize_record_with_one()
  {
    Record* temp = new Record[rcnSize_+1];
    for(int i = 0; i < rcnSize_; ++i)
    {
      temp[ i ] = rc_[ i ];
    }
    delete[] rc_;
    rc_ = temp;
    ++rcnSize_;
  }
};

#endif // RENTALMANAGER_H

Code:

#include "RentalManager.h"
#include <iostream>


using namespace std;

int main(int argc, char *argv[])
{
  RentalManager RM;
  RM.add_new_record();
  cout<<endl;
  RM.add_new_record();
  
  cout<<endl;
  cout<<"*** current status ***"<<endl;
  RM.view_all_actions();
  cout<<endl;
  
  return 0;
}

fp_unit · 02-10-2005, 11:28 AM

Thanks alot Valmont, I'll check it out when I get home.