Array Object - Terminating Compiler

I have been practicing C++ for a month now. I'm in the pace of programming array of objects. The compiler is successfully compiling the program but the problem is after I input the first element for the array of object the compiler is terminated. I end up reopening the compiler and debug whatever the logical error is. I've spent a week debugging the issue but I can't understand what's happening.

Code:

#include <iostream.h>
#include <conio.h>
#include <stdio.h>

class Product
{
   private:  //private by default
	char *productName;
	char *brandName;
	float price;

   public:
	Product( void );
	~Product( void );
	Product( char *p, char *b, float pr );
	void setProductName( char *prodName );
	char *getProductName();
	void setBrandName( char *brand );
	char *getBrandName();
	void setPrice( float p );
	float getPrice();
	void displayProducts();
};

Product::Product()
{
  productName = "";
  brandName = "";
  price = 0;
}

Product::Product( char *p, char *b, float pr )
{
  productName = p;
  brandName = b;
  price = pr;
}

Product::~Product()
{
  delete productName;
  delete brandName;
}

void Product::setProductName( char *prodName )
{
   productName = prodName;
}

char* Product::getProductName()
{
   return productName;
}

void Product::setBrandName( char *brand )
{
   brandName = brand;
}

char* Product::getBrandName()
{
   return brandName;
}

void Product::setPrice( float p )
{
   price = p;
}

float Product::getPrice()
{
   return price;
}

void Product::displayProducts()
{
   cout << "Product Name: " << getProductName() << endl;
   cout << "Brand Name  : " << getBrandName() << endl;
   cout << "Price       : " << getPrice() << endl;
}

//MAIN FUNCTION
int main( void )
{
  const char SIZE = 3;
  char product[ 30 ];
  char brand[ 30 ];
  float price;
  Product prod[ SIZE ] = { Product( "TestProduct1", "TestBrand1", 1 ),
			   Product( "TestProduct2", "TestBrand2", 2 ),
			   Product( "TestProduct3", "TestBrand3", 3 ) };
  clrscr();


  /*for( int counter = 0; counter < SIZE; counter++ )
  {
     cout << "Product: "; cin.getline( product, 30 );
     cout << "Brand  : "; cin.getline( brand, 30 );
     cout << "Price  : "; cin >> price;

     prod[ counter ] = Product( product, brand, price  );
     cout << endl;
  }*/

  /*for( int counter1 = 0; counter1 < SIZE; counter1++ )
  {
     cout << "Record [ " << (counter1+1) << " ]:\n\n ";
     prod[ counter1 ].displayProducts();
  }*/

  prod[ 0 ].displayProducts();
  prod[ 1 ].displayProducts();
  prod[ 2 ].displayProducts();

  delete[] prod;
  delete[] product;
  delete[] brand;
  //getch();
  cin.get();
  return 0;
}

Even if you remove the comment to the loops you will still have the same problem. I hope you could help and explain to me what's really happening. thanks in advance forum members.

The compiler that I'm using is pretty old so it doesn't have any updated header.

Compiler: Turbo C++ 3.0

Forgive my brevity, but you are sending character pointers around without copying the actual strings. The issue is that your destructor is trying to delete data it no longer can delete. Try one of two things. Either replace all of your "str1= str2" with strcpy calls, OR use std::string. std::string is beautiful.
Also, be mindful of the difference between delete and delete[].

I've tried to change the code as what you've advised, but it's doing the same thing. it's closing or terminating my compiler.

Code:

#include <iostream.h>
#include <string.h>
#include <conio.h>
#include <stdio.h>

class Product
{
   private:  //private by default
	char *productName;
	char *brandName;
	float price;

   public:
	Product( void );
	~Product( void );
	Product( char *p, char *b, float pr );
	void setProductName( char *prodName );
	char *getProductName();
	void setBrandName( char *brand );
	char *getBrandName();
	void setPrice( float p );
	float getPrice();
	void displayProducts();
};

Product::Product()
{
  productName = "";
  brandName = "";
  price = 0;
}

Product::Product( char *p, char *b, float pr )
{
  strcpy( productName, p );
  strcpy( brandName, b );
  price = pr;
}

Product::~Product()
{
  delete productName;
  delete brandName;
}

void Product::setProductName( char *prodName )
{
   strcpy( productName, prodName );
}

char* Product::getProductName()
{
   return productName;
}

void Product::setBrandName( char *brand )
{
   strcpy( brandName, brand );
}

char* Product::getBrandName()
{
   return brandName;
}

void Product::setPrice( float p )
{
   price = p;
}

float Product::getPrice()
{
   return price;
}

void Product::displayProducts()
{
   cout << "Product Name: " << getProductName() << endl;
   cout << "Brand Name  : " << getBrandName() << endl;
   cout << "Price       : " << getPrice() << endl;
}

//MAIN FUNCTION
int main( void )
{
  const char SIZE = 3;
  char product[ 30 ];
  char brand[ 30 ];
  float price;
  Product prod[ SIZE ] = { Product( "Product1", "Brand1", 1 ),
			   Product( "Product2", "Brand2", 2 ),
			   Product( "Product3", "Brand3", 3 )};
  clrscr();

  /*for( int counter = 0; counter < SIZE; counter++ )
  {
     cout << "Product: "; cin.getline( product, 30 );
     cout << "Brand  : "; cin.getline( brand, 30 );
     cout << "Price  : "; cin >> price;

     prod[ counter ] = Product( product, brand, price  );
     cout << endl;
  }*/

  for( int counter1 = 0; counter1 < SIZE; counter1++ )
  {
     cout << "Record [ " << (counter1+1) << " ]:\n\n ";
     prod[ counter1 ].displayProducts();
  }

  delete[] prod;
  delete[] product;
  delete[] brand;
  //getch();
  cin.get();
  return 0;
}

Note that productName and brandName are just pointers. As such, your use of strcpy merely copies the strings into nowhere (or rather, into memory that does not belong to you). This results in undefined behaviour, one of the symptoms of which is the crash that you observed (though I presume it is a crash of the program, not the compiler, as your compiler should be more robust than that).

You need to do dynamic memory allocation (and clearly you are trying to, since you used delete in the destructor). This means using new[] and delete[] (not delete, as you are working with a dynamically allocated array of char). Also, you should not be assigning empty string literals to productName and brandName as you would then not be allowed to use delete[] on them. You might want to initialise them to point to the first char of dynamically allocated empty strings.

Along the same lines, you should not be using delete[] on prod, product and brand in main() since they do not involve dynamic memory allocation with new[].

Note that now that you are doing all this memory management, you not only need to implement the destructor, but need to implement the copy constructor and copy assignment operator. Now do you see why CodeMonkey suggested that you use std::string instead? (Not that std::string is beautiful though: that class has design issues.)

Be aware of const-correctness: member functions that are getters/observers like getProductName() should be declared const since they do not change the logical state of the object.

EDIT:
Oh, and note that <iostream.h>, <string.h>, and <stdio.h> should be <iostream>, <cstring> and <cstdio> respectively, though if you use std::string then change <cstring> to <string>. The use of these standard headers would mean that you either use a using directive/declaration like using namespace std; or you prefix names in the standard namespace with std:: , e.g., std::cout.

Also, you only use the stuff in the non-standard <conio.h> for clrscr(), so you might want to consider if you really need it (you could just print out a bunch of newlines, for example).

thanks for all the advice. As much as I would like to use the updated header and classes in C++, I can't, because this is not what the school require. We have to make the string as what you can see in my code. I'll be modifying my code.. I'll repost it if it's fixed or if there's any problem.

Originally Posted by $l4xklynx

As much as I would like to use the updated header and classes in C++, I can't, because this is not what the school require.

<string.h> and <stdio.h> are arguably fine since they are standard C headers. <iostream.h>, on the other hand, is pre-standard. I cannot even include it on Visual C++ 2005 Professional with language extensions enabled. An easy workaround that avoids changing too much of the code given by your teacher is to use using directives in your source files, e.g.,

Code:

#include <iostream>
using namespace std; // <-- using directive so that you do not need to fully qualify std::cout.

int main()
{
    cout << "Hello world!" << endl;
    return 0;
}

Originally Posted by $l4xklynx

We have to make the string as what you can see in my code.

As a student, that is fine since you will have to learn about proper memory management in C++ eventually.

As much as I would like to use the updated header and classes in C++, I can't, because this is not what the school require.

That's simply not acceptable. The last three versions of Visual Studio have not even distributed the pre-standard headers. Just about every halfway recent compiler (meaning released later than 2000) either warns on their usage or fails to compile completely. They are a relic that is over 10 years old.
How can the school justify teaching with such incredibly outdated tools?

Originally Posted by CornedBee

How can the school justify teaching with such incredibly outdated tools?

I can think of a few reasons why they might do this:

1. they're lazy.
2. they don't want to spend money that they think they don't have to. their likely thought process is "it says C++, it must be the same thing."
3. they don't know that there are newer standards for the language. If this is the case, you should inform them.
4. they don't care.

If you have an instructor who insists that you use turboc++ 3.0 instead of something newer, he's the problem. Get a newer compiler, like MSVC++ 2008 express edition (it's free) and turn in your homework, written in standards-compliant C++ code, and make notes to that effect. if the teacher doesn't accept it then you know he's living in the dark ages, and doesn't care whether you are taught the right way.

Also, an integrated development environment (editor, etc.) is not the same thing as a compiler. it would be good for you to familiarize yourself with the correct terminology. when I opened this thread, I was expecting to see code that caused the compiler to crash while compiling. the compiler is the program that converts your C++ code to executable code (in simplest terms) when you click the compile button in your editor.

Of course, if joining such a course, it is better to complain that is old and outdated and then drop the course, because the last thing the industry needs are programmers who cannot write standards compliant code.
Even worse is when they come trying to code their out-dated code in a newer compiler on the work, typically Visual Studio, and finds to their horror that it does not work or does not compile.

Originally Posted by CornedBee

That's simply not acceptable. The last three versions of Visual Studio have not even distributed the pre-standard headers. Just about every halfway recent compiler (meaning released later than 2000) either warns on their usage or fails to compile completely. They are a relic that is over 10 years old.
How can the school justify teaching with such incredibly outdated tools?

It is truly ridiculous, I myself is perplexed with the way they provide information to their students. However, I'm just thinking that maybe they are just providing the fundamentals and it's up to us to maximize the knowledge about what they've taught us. I cannot even use the book I bought because it uses the updated codings in C++. So I'm totally outdated, but, despite all of that, I research on my own and keep myself updated. I remember last week I don't even know how to use getline() I till use gets() from C. I don't even know how linker works, I don't even know how to create my own header, however, I will start researching about those stuff. I hope I could find a better book about C++.

Originally Posted by elkvis

1. they're lazy.
2. they don't want to spend money that they think they don't have to. their likely thought process is "it says C++, it must be the same thing."
3. they don't know that there are newer standards for the language. If this is the case, you should inform them.
4. they don't care.

I think they have all of the above. It' a good thing that I joined this forum and I found a bigger world in C and C++. I never thought that there are tons of things that I have to research on my own. I guess it's up to me to find out the things in C and C++.

Code:

#include <iostream.h>
#include <string.h>
#include <conio.h>
#include <stdio.h>

class Product
{
   private:  //private by default
	char *productName;
	char *brandName;
	float price;

   public:
	Product( void );
	~Product( void );
	Product( char *p, char *b, float pr );
	void setProductName( char *prodName );
	char *getProductName();
	void setBrandName( char *brand );
	char *getBrandName();
	void setPrice( float p );
	float getPrice();
	void displayProducts();
};

/*Product::Product()
{
  productName = new char[ 0 ];
  strcpy( productName, "" );
  brandName = new char[ 0 ];
  strcpy( brandName, "" );
  price = 0;
}*/

Product::Product( char *p, char *b, float pr )
{
  productName = new char[ strlen( p ) + 1 ];
  strcpy( productName, p );

  brandName = new char[ strlen( b ) + 1 ];
  strcpy( brandName, b );

  price = pr;
}

Product::~Product()
{
  delete[] productName;
  delete[] brandName;
}

void Product::setProductName( char *prodName )
{
   productName = new char[ strlen( prodName ) + 1 ];
   strcpy( productName, prodName );
}

char* Product::getProductName()
{
   return productName;
}

void Product::setBrandName( char *brand )
{
   brandName = new char[ strlen( brand ) + 1 ];
   strcpy( brandName, brand );
}

char* Product::getBrandName()
{
   return brandName;
}

void Product::setPrice( float p )
{
   price = p;
}

float Product::getPrice()
{
   return price;
}

void Product::displayProducts()
{
   cout << "Product Name: " << getProductName() << endl;
   cout << "Brand Name  : " << getBrandName() << endl;
   cout << "Price       : " << getPrice() << endl;
}

//MAIN FUNCTION
int main( void )
{
  const char SIZE = 3;
  //char product[ 30 ];
  //char brand[ 30 ];
  //float price;
  Product prod[ SIZE ] = { Product( "Product1", "Brand1", 1 ),
			   Product( "Product2", "Brand2", 2 ),
			   Product( "Product3", "Brand3", 3 )};
  //Product prod[ SIZE ];

  clrscr();

  /*for( int counter = 0; counter < SIZE; counter++ )
  {
     cout << "Product: "; cin.getline( product, 30 );
     cout << "Brand  : "; cin.getline( brand, 30 );
     cout << "Price  : "; cin >> price;

     prod[ counter ] = Product( product, brand, price  );
     cout << endl;
  }*/

  for( int counter1 = 0; counter1 < SIZE; counter1++ )
  {
     cout <<"Record [ " << (counter1+1) << " ]:\n\n ";
     prod[ counter1 ].displayProducts();
  }

  //delete[] prod
  //delete[] product
  //delete[] brand
  cin.get();
  return 0;
}

I was able to fix the problem somehow, however I cannot get the idea on how to initialize my constructor. so that I could just create an object of the class without assigning something to the constructor like: Product prod;

Without this void constructor, I cannot make any object of the class as the example above.

Code:

/*Product::Product()
{
  productName = new char[ 0 ];
  strcpy( productName, "" );
  brandName = new char[ 0 ];
  strcpy( brandName, "" );
  price = 0;
}*/

This is how I fixed the problem, I don't really know how to explain it properly but as to my undersatnding the *productName before is not pointing to something: productName = prodName, am I right? because a pointer must be initialized before you can use it. The productName has dynamic allocation using the new keyword, where the size of the memory it's going to take is the size of the string that the prodName will take, plus 1 for the null that will terminate the string, is that correct? The characters that prodName received is then copied by the instance variable productName.

Code:

void Product::setProductName( char *prodName )
{
   productName = new char[ strlen( prodName ) + 1 ];
   strcpy( productName, prodName );
}

I was able to change the destructor for the productName and brandName pointer. I undersatnd that if you declared dynamic memory allocation using the "new with []" you will use the "delete with []" but if just the: "new without []" then you will use the "delete" without the brackets, but , honestly I don't know the difference between "new with []" and just "new without []"; "delete with []" and "delete without []". What is "new with []" and "new without []". I will still research on that.

Code:

Product::~Product()
{
  delete[] productName;
  delete[] brandName;
}

Please correct me if I have something wrong in my explanation and understanding.

Originally Posted by $l4xklynx

This is how I fixed the problem, I don't really know how to explain it properly but as to my undersatnding the *productName before is not pointing to something: productName = prodName, am I right? because a pointer must be initialized before you can use it. The productName has dynamic allocation using the new keyword, where the size of the memory it's going to take is the size of the string that the prodName will take, plus 1 for the null that will terminate the string, is that correct? The characters that prodName received is then copied by the instance variable productName.

You seem to have the mind of a C dev, which I cannot say if it good or bad. But since this is C++, you should probably be thinking in more of C++ terms.
A pointer is simply a variable containing a memory address. It is a type in itself.
However, everything you wish to store must have some reserved memory, yes? A variable is automatically reserved memory by the compiler.
So then you will need to ask the operating system to reserve enough memory for you. The operating systems returns a pointer to that, so you assign it to a pointer.

Although, it may be better to think of it as reserving a number of elements of a certain type. When you call new, you say that you want to reserve memory for X elements of type T.

Other than that - you are correct. Strlen will return the number of characters in a string (excluding the '\0'), so you need the length + 1 to store it.

Oh and because you are researching, here is a little on T* vs T *: http://www.research.att.com/~bs/bs_faq2.html#whitespace

I was able to change the destructor for the productName and brandName pointer. I undersatnd that if you declared dynamic memory allocation using the "new with []" you will use the "delete with []" but if just the: "new without []" then you will use the "delete" without the brackets, but , honestly I don't know the difference between "new with []" and just "new without []"; "delete with []" and "delete without []". What is "new with []" and "new without []". I will still research on that.

New says allocate 1 element of type T. New[] says allocate n elements of type T.
They are different, and therefore, they have match deletes, too.
Delete says this is pointer that points to a single element. Delete[] says it's a pointer pointing to n elements.

As for the constructor, it needs to take arguments to everything you pass to it. If you are going to pass char*, char*, int, then that constructor must take at least those 3 parameters.
And "void" in the function head when it takes no other parameters is a relic from C and is not needed in C++.

Originally Posted by $l4xklynx

This is how I fixed the problem, I don't really know how to explain it properly but as to my undersatnding the *productName before is not pointing to something: productName = prodName, am I right? because a pointer must be initialized before you can use it.

Yes, a pointer must point to an object before you can access the object that it points to. However, you actually should use new char[1], not new char[0], since the empty string actually consists of the null character. Consequently, a simple assignment will do instead of using strcpy().

Originally Posted by $l4xklynx

The productName has dynamic allocation using the new keyword, where the size of the memory it's going to take is the size of the string that the prodName will take, plus 1 for the null that will terminate the string, is that correct? The characters that prodName received is then copied by the instance variable productName.

Yes. However, you forgot to delete[] the existing productName before setting it to the new one.

Actually, if you are interested in exception safety (but this may be too advanced for you at the moment, so I'd say its optional), you should first attempt to create the new string, and only then destroy the old one since new[] may throw a std::bad_alloc exception if space cannot be allocated. Ideally, the state of the object should not change if an exception is thrown, hence you should only destroy the old string if no exception is thrown, e.g.,

Code:

void Product::setProductName( const char* prodName )
{
   char* temp = new char[ strlen( prodName ) + 1 ];
   delete[] productName;
   productName = temp;
   strcpy( productName, prodName );
}

Note that I declared prodName as a const char* since you will not be changing its contents.

[quote=$l4xklynx]I was able to change the destructor for the productName and brandName pointer.[/url]
That's good. Remember to also implement the copy constructor and copy assignment operator so that they copy not just the pointers but the entire strings pointed to as well.

Originally Posted by $l4xklynx

I undersatnd that if you declared dynamic memory allocation using the "new with []" you will use the "delete with []" but if just the: "new without []" then you will use the "delete" without the brackets, but , honestly I don't know the difference between "new with []" and just "new without []"; "delete with []" and "delete without []". What is "new with []" and "new without []".

You just need to remember that new is used for a lone object, but new[] is used for an array of objects.

The reason that C++ differentiates the array new/delete is that if you allocate a bunch of objects, you would need to construct those objects by calling the constructor for each instance, and when you delete those objects should be destroyed by calling the destructor for each instance. If you just allocate using new and delete, it is assumed that you are allocating a single object.

Sure, basic types may not have any constructor/destructor, but that's a different matter.

It may well (must?) be that new[] stores some sort of information about the number of objects allocated, which delete[] uses to know how many calls to the destructor it should make. If that information makes an additional element to the size of the object, then calling delete without [] when you created it with [] will cause a problem. This MAY OR MAY NOT be the case in a particular implementation of new/delete, so what you are doing may work in one case, and then you compile it on a different system and it breaks - which is bad.

--
Mats