Need help...prog crashes sometime

[losh177]

Hi, I'm having some issues with my homework. It is a polymorphism homework. IFrom the console it takes two string (title and author/performer) a char (format for records) and a call number (int). Currently it compiles fine with some warning for strcpy. I have used the same data and sometimes it crashes after entering the int at any of the 5 times it is asked, others it just goes without any issues

//holding.h

#ifndef _HOLDING_H
#define _HOLDING_H

#include <iostream>
#include <string.h>
#include <stdlib.h>
#include <string.h>

class Holding
{
	protected:
		char* title;
		int id;
		//ostream os;

	public:
		//constructor
		Holding()
		{
			title = new char[1];
			title[0] = '\0';
			id = 0;
		}

		Holding(const Holding&);
	public:
		//set data
		void setTitle(std::string);

		void setID(int i)
		{
			id = i;
		}

		//call data
		char* getTitle() const
		{
			return title;
		}

		int getId() const
		{
			return id;
		}

		//virtual print function
		virtual void Print() = 0;

		//destructor
		virtual ~Holding()
		{
			delete [] title;
		}
};
#endif

//holding.cpp

#include "holding.h"
#include <iomanip>
#include <fstream>
#include <stdlib.h>
#include <string.h>

using namespace std;

extern ofstream csis;

//copy coonstructor
Holding::Holding(const Holding& Hold)
{
	//copy title
	title = new char[strlen(Hold.getTitle() + 1)];
	strcpy(title, Hold.getTitle());
	title[strlen(Hold.getTitle())] = '\0';

	//copy ID
	id = Hold.getId();
}

//set data methods
void Holding::setTitle(string str)
{
	delete [] title;

	char *buf;

	buf = new char[str.length() + 1];
	title = new char[str.length() + 1];

	for (int i = 0; i < str.length(); i++)
		buf[i] = str[i];

	strcpy(title, buf);
	title[str.length()] = '\0';

	delete [] buf;
}

//book.h

#ifndef _BOOK_H
#define _BOOK_H

#include "Holding.h"
#include <iostream>
#include <stdlib.h>
#include <string.h>

class Book : public Holding
{
	private:
		char* author;
		
	public:
		//constructor
		Book():Holding() 
		{
			author = new char[1];
			author[0] = '\0';
		}

		Book(const Book& Bk):Holding(Bk)
		{
			author = new char[strlen(Bk.getAuthor() + 1)];
			strcpy(author, Bk.getAuthor());
			author[strlen(Bk.getAuthor())] = '\0';
		}
		
		//destructor
		virtual ~Book()
		{
			delete [] author;
		}

		//virtual method
		virtual void Print();
		
		//set data
		void setAuthor(std::string);

		//get data
		char* getAuthor() const
		{
			return author;
		}
};
#endif

//book.cpp

#include "Book.h"
#include <iomanip>
#include <fstream>
#include <stdlib.h>
#include <string.h>

using namespace std;

extern ofstream csis;

void Book::setAuthor(string str)
{
	char* buf;
	buf = new char[str.length() + 1];
	author = new char[str.length() + 1];

	for (int i = 0; i < str.length(); i++)
		buf[i] = str[i];

	strcpy(author, buf);

	author[str.length()] = '\0';

	delete [] buf;
}

void Book::Print()
{
	cout << "BOOK: " << getAuthor() << " \"" << getTitle() << "\" " << getId() << endl;
	csis << "BOOK: " << getAuthor() << " \"" << getTitle() << "\" " << getId() << endl;
}

//record.h

#ifndef _RECORD_H
#define _RECORD_H

#include "holding.h"
#include <iostream>
#include <string.h>
#include <stdlib.h>

static const char *RecFormat[4] = {"LP", "Cassette", "Reel_to_reel", "Disk"};

class Record : public Holding
{
	private:
		char* performer;
		int index;
		
	public:
		//constructor
		Record():Holding() 
		{
			index = 0;
			performer = new char[1];
			performer[0] = '\0';
		}

		Record(const Record& Rec):Holding(Rec)
		{
			performer = new char[strlen(Rec.getPerformer() + 1)];
			strcpy(performer, Rec.getPerformer());
			performer[strlen(Rec.getPerformer())] = '\0';
			index = Rec.index;
		}
		
		//destructor
		virtual ~Record()
		{
			delete [] performer;
		}

		//virtual method
		virtual void Print();
		
		//set data
		void setPerformer(std::string);
		void setFormat(const char);
		
		//get data
		char* getPerformer() const
		{
			return performer;
		}
		
		const char* getFormat()
		{
			return RecFormat[index];
		}
};
#endif;

//record.cpp

#include "record.h"
#include <iomanip>
#include <fstream>
#include <stdlib.h>

using namespace std;

extern ofstream csis;

void Record::setPerformer(string per)
{
	char* buf;
	buf = new char[per.length() + 1];
	performer = new char[per.length() + 1];

	for (int i = 0; i < per.length(); i++)
		buf[i] = per[i];

	strcpy(performer, buf);

	performer[per.length()] = '\0';

	delete [] buf;
}

//set index
void Record::setFormat(const char f)
{
	if (f == 'l' || f == 'L')
		index = 0;
	else if (f == 'c' || f == 'C')
		index = 1;
	else if (f == 'r' || f == 'R')
		index = 2;
	else if (f == 'd' || f == 'D')
		index = 3;
}

//virtual print
void Record::Print()
{
	cout << "RECORDING: \"" << getTitle() << "\" " << getPerformer() << " (" << getFormat() << ") " << getId() << endl;
	csis << "RECORDING: \"" << getTitle() << "\" " << getPerformer() << " (" << getFormat() << ") " << getId() << endl;
}

// library.h

#ifndef _STRDRV_H
#define _STRDRV_H

#include "book.h"
#include "record.h"

int main();

#endif

// Strdrv.cpp

#include <iostream>
#include <string>
#include <fstream>
#include <stdlib.h>
#include "library.h"

using namespace std;

ofstream csis;

int main() 
{
	csis.open("csis.dat");
    char type;
	char format;
	string data;
	int id;


	cout << "Enter holdings to be stored in a list:" << endl;
	csis << "Enter holdings to be stored in a list:" << endl;

	cout << endl;
	csis << endl;

	const int MAX = 5;

	Holding *Hold[MAX];
	Book Bk;
	Record Rec;

	//loop to get 5 entries
	for (int i = 0; i < MAX; i++)
	{
		cout << "Enter B for book, R for recording: ";
		cin >> type;
		cin.ignore(1, '\n');

		//base on type selected excecute as book or record
		switch(type)
		{
			case 'B':
			case 'b':
				
				//retrieve data
				cout << "Enter book title: ";
				getline(cin, data);
				Bk.setTitle(data);
				csis << "Enter book title: " << data << endl;

				cout << "Enter book author: ";
				getline(cin, data);
				Bk.setAuthor(data);
				csis << "Enter book author: " << data << endl;

				//set holding pointer
				Hold[i] = new Book(Bk);

				break;

			case 'R':
			case 'r':

				cout << "Enter recording title: ";
				getline(cin, data);
				Rec.setTitle(data);
				csis << "Enter recording title: " << data << endl;

				cout << "Enter performer: ";
				getline(cin, data);
				Rec.setPerformer(data);
				csis << "Enter performer: " << data << endl;
				
				cout << "Enter format: (L)P, (C)assete, (R)ee_toreel, (D)isk: ";
				cin >> format;
				Rec.setFormat(format);
				csis << "Enter format: (L)P, (C)assete, (R)ee_toreel, (D)isk: " << format << endl;

				//set holding pointer
				Hold[i] = new Record(Rec);
				
				break;
		}

		cout << "Enter call number: ";
		cin >> id;
		csis << "Enter call number: " << id << endl;

		Hold[i]->setID(id);
	}

	cout << endl;
	csis << endl;

	cout << "Here are the holdings:" << endl;
	csis << "Here are the holdings:" << endl;

	cout << endl;
	csis << endl;

	for (int c = 0; c < MAX; c++)
	{
		Hold[c]->Print();
	}
	
    csis.close();
}

This is the output/input I've used on every single time (crash/no-crash)

Enter holdings to be stored in a list:
 
Enter B for book, R for recording: B
Enter book title: The C++ Programming Language
Enter book author: Stroustrup, Bjarne
Enter call number: 101
Enter B for book, R for recording: R
Enter recording title: Abbey Road
Enter performer: The Beatles
Enter format: (L)P, (C)assette, (R)eel_to_reel, (D)isk: C
Enter call number: 401
Enter B for book, R for recording: R
Enter recording title: Lone Star State of Mind
Enter performer: Nanci Griffith
Enter format: (L)P, (C)assette, (R)eel_to_reel, (D)isk: D
Enter call number: 402
Enter B for book, R for recording: B
Enter book title: Object‑Oriented Design
Enter book author: Booch, Grady
Enter call number: 102
Enter B for book, R for recording: B
Enter book title: The C++ Primer
Enter book author: Lippman, Stanley
Enter call number: 103

Here are the holdings:
 
BOOK: Stroustrup, Bjarne "The C++ Programming Language" 101
RECORDING: "Abbey Road" The Beatles (Cassette) 401
RECORDING: "Lone Star State of Mind” Nanci Griffith (CD) 402
BOOK: Booch, Grady "Object‑Oriented Design" 102
BOOK: Lippman, Stanley "The C++ Primer" 103

Thanks in advance

[mike_2000_17]

Here is what my eagle eye has found:

At line 17 of holding.cpp
At line 26 of book.h
At line 30 of record.h

In all those cases, you have the same misplaced parenthesis:

//you wrote:
title = new char[strlen(Hold.getTitle() + 1)];
//when it should be:
title = new char[strlen(Hold.getTitle()) + 1]; //notice the displaced closing parenthesis.

That would certainly explain the fact that it crashes not all the time, because this just introduces a few bytes of misalignment (writing memory where it shouldn't) and often it's not enough to cause a crash.

Save yourself all that trouble with copying C-strings, by using the C++ string class instead, like you do already in main().

[losh177]

thank you eagle eye mike!!!....i hate those type of errors, i cannot believe i miss placed the parenthesis....i know it will make life easier to use string class reather than char* but thanks to the teach we cannot

[losh177]

hi, i just got the same problem after changing the following lines:

holding.h line 49: virtual std::ostream& Print(ostream& os); book.h line 38:

virtual std::ostream& Print(std::ostream& os)
{
	os << "BOOK: " << getAuthor() << " \"" << getTitle() << "\" " << getId() << std::endl;
	return os;
}

record.h line 43

virtual std::ostream& Print(std::ostream& os)
{
	os << "BOOK: " << getAuthor() << " \"" << getTitle() << "\" " << getId() << std::endl;
	return os;
}

any ideas....i also changed the book and record objects in library.cpp to pointers, but then change them back.

[losh177]

i got.....i forgot to clear the pointers before assigning them a value