dynamic array and losing data of the first element

This is a discussion on dynamic array and losing data of the first element within the C++ Programming forums, part of the General Programming Boards category; Hello! Here's my problematic code: Code: class konto { private: char *nazwa; int size; papier *tabl; public: ~konto(); konto(char *); ...

  1. #1
    Registered User
    Join Date
    Feb 2010
    Posts
    9

    dynamic array and losing data of the first element

    Hello!

    Here's my problematic code:

    Code:
    class konto
    {
    private:
    	char *nazwa;
    	int size;
    	papier *tabl;
    public:
    	~konto();
    	konto(char *);
    	void dodajPapier(papier &);
    	void zapiszWplik(const char *nazwaPliku);
    };
    
    konto::konto(char *nazwa)
    {
    	this->nazwa = new char [len(nazwa)];
    	strcpy(this->nazwa, nazwa);
    
    	this->size = 0;
    	tabl = new papier[size+1];
    }
    
    konto::~konto()
    {
    	delete [] tabl;
    }
    
    void konto::dodajPapier(papier &akcja)
    {                                                    //first element in the array still exists
    	tabl[size] = akcja;          
                                                         //here tabl[0] points somewhere else but not to 0-element
    
    	papier *temp = new papier[++size];
    
    	for(int i = 0; i < size; i++)
    		temp[i] = tabl[i];
    
    	tabl = temp;
    
    }
    When I add second object of papier the value of tabl[0] is lost.. Next objects are created properly in the array.
    Why's that ? I can't fix it even though I know where it happens :-/
    Last edited by fx69; 02-24-2010 at 09:20 AM. Reason: post syntax

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,396
    At a glance, it seems that:
    • Your destructor does not take care of nazwa.
    • Your class lacks a correctly implemented copy constructor and copy assignment operator.
    • The constructor should have a const char* parameter.
    • dodajPapier should have a const reference parameter.
    • In dodajPapier, you fail to destroy the existing array before assigning temp to tabl.
    • Expanding the dynamic array each time an element is added is slow.


    These problems can be solved by changing nazwa to be a std::string and tabl to be a std::vector<papier>. You can then do away with the size member variable. In the process, the problem that you observed may also disappear.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Registered User
    Join Date
    Feb 2010
    Posts
    9
    Thank you for your reply
    Your class lacks a correctly implemented copy constructor and copy assignment operator.
    Class papier has these:
    Code:
    papier::papier(const papier &akcja)
    {
    	this->nazwa = new char [20];
    	::strcpy(nazwa, akcja.nazwa);
    	this->cenaKup = akcja.cenaKup;
    	this->cenaSprz = akcja.cenaSprz;
    	this->ilosc = akcja.ilosc;
    	this->prowizja = akcja.prowizja;
    }
    
    papier& papier::operator =(const papier &akcja)
    {
    	this->nazwa = new char [20];
    	::strcpy(nazwa, akcja.nazwa);
    	this->cenaKup = akcja.cenaKup;
    	this->cenaSprz = akcja.cenaSprz;
    	this->ilosc = akcja.ilosc;
    	this->prowizja = akcja.prowizja;
    }
    
    papier::~papier()
    {
    	delete [] nazwa;
    	nazwa = NULL;
    }
    and the default constructor also.
    I think something is wrong with deleting [] tabl, but when I put it from the destructor to function dodajPapier like this:
    Code:
    delete [] tabl;
    tabl = temp;
    I get some memory crash error
    ps: I followed your suggestion and fixed:
    * Your destructor does not take care of nazwa.
    * The constructor should have a const char* parameter.
    * dodajPapier should have a const reference parameter.
    Last edited by fx69; 02-24-2010 at 10:19 AM. Reason: ps.

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,396
    Quote Originally Posted by fx69
    Class papier has these:
    Good to hear, but I was talking about konto

    Quote Originally Posted by fx69
    I think something is wrong with deleting [] tabl
    I suspect that the problem lies with papier's copy assignment operator, e.g., it performs shallow copies when it should perform deep copies. Again, if you make use of the standard library, you can eliminate much of this error prone memory management.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  5. #5
    Registered User
    Join Date
    Feb 2010
    Posts
    9
    I know I could use vectors and strings but I'd like to practice of pointers That's generally why I drain this code so much
    What do you mean saying: 'shallow/deep copies' ?

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,396
    Quote Originally Posted by fx69
    What do you mean saying: 'shallow/deep copies' ?
    With respect to pointers, a shallow copy involves copying only the pointer, whereas a deep copy copies the entire dynamic array.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    spurious conceit MK27's Avatar
    Join Date
    Jul 2008
    Location
    segmentation fault
    Posts
    8,300
    Forgive me for butting in but don't you only have to use "this" if you have a parameter name matching a member name?
    C programming resources:
    GNU C Function and Macro Index -- glibc reference manual
    The C Book -- nice online learner guide
    Current ISO draft standard
    CCAN -- new CPAN like open source library repository
    3 (different) GNU debugger tutorials: #1 -- #2 -- #3
    cpwiki -- our wiki on sourceforge

  8. #8
    Registered User
    Join Date
    Feb 2010
    Posts
    9
    Code:
    papier::papier(const papier &akcja)
    {
    	delete this->nazwa;
    
    	this->nazwa = new char [len(akcja.nazwa) + 1];
    	::strcpy(this->nazwa, akcja.nazwa);
    
    	this->cenaKup = akcja.cenaKup;
    	this->cenaSprz = akcja.cenaSprz;
    	this->ilosc = akcja.ilosc;
    	this->prowizja = akcja.prowizja;
    }
    
    papier & papier::operator =(const papier &akcja)
    {
    	if(this != &akcja) 				//if object's the same: A = A
    	{
    		delete this->nazwa;
    
    		this->nazwa = new char [len(akcja.nazwa) + 1];
    		//for(int i=0; i<len(akcja.nazwa); i++)
    		//	this->nazwa[i] = akcja.nazwa[i];
    
    		::strcpy(this->nazwa, akcja.nazwa);
    
    		this->cenaKup = akcja.cenaKup;
    		this->cenaSprz = akcja.cenaSprz;
    		this->ilosc = akcja.ilosc;
    		this->prowizja = akcja.prowizja;
    	}
    
    	return *this;
    }
    Is this what I need ?
    But still doesn't work :/

  9. #9
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,396
    Quote Originally Posted by fx69
    Is this what I need ?
    I cannot say yes or no without knowing the definition of papier. What I can say is that the copy assignment operator is not exception safe since you destroy the existing C-style string before you have created the new one (you have no guarantee that the new one can be created). You certainly do not need to destroy the existing C-style string in the copy constructor since it does not exist, and in fact the pointer may contain garbage at that point.

    If you want to practice the use of manual memory management, then I suggest that you stop what you are doing now and start working on a string class and/or dynamic array of int class. Clearly, you are doing more then you can handle at the moment, i.e., you are trying to write a class that has both a dynamic string and dynamic array, and the dynamic array is of objects that are themselves written by you to use manual memory management. There are too many things that can go wrong.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  10. #10
    Registered User
    Join Date
    Feb 2010
    Posts
    9
    You are right I just didn't think that the project would get beyond me so much..
    However it's only the first element of the array which data is lost in a certain place so I'll keep on trying to figure out what's wrong.
    Here's my almost whole def of class 'papier':
    Code:
    class papier
    {
    private:
    	unsigned int ilosc;
    	float cenaKup, cenaSprz, prowizja;
    	char kierunek;
    public:
    	char *nazwa;				//will be private ! ! !
    
    	//constructors
    	papier (const papier &);		//copy constructor
    	papier (char *, char, int, float, float, float);
    	papier ();
    
    	papier &operator=(const papier &);			//overloaded operator
    
    	string show();
    	float wartosc();
    	float wartosc(float);
    
    	~papier();
    };
    
    papier::papier()
    {
    	this->cenaKup = 0;
    	this->cenaSprz = 0;
    	this->ilosc = 0;
    	this->kierunek = NULL;
    	this->nazwa = NULL;
    	this->prowizja = 0;
    }
    
    papier::papier(char *nazwa, char kierunek, int ilosc, float cenaKup, float cenaSprz, float prowizja)
    {
    	if(ilosc < 0 || cenaKup < 0 || cenaSprz < 0)
    		cout << "Wartości nie mogą być mniejsze od 0\n";
    	else
    	{
    		this->cenaKup = cenaKup;
    		this->cenaSprz = cenaSprz;
    		this->ilosc = ilosc;
    	}
    
    	if(len(nazwa) < 20)
    	{
    		this->nazwa = new char [20];
    		::strcpy(this->nazwa, nazwa);
    	}
    	else
    		cout << "Zbyt długa nazwa papieru\n";
    
    	if(kierunek == 'K' || kierunek == 'S')
    		this->kierunek = kierunek;
    	else
    		cout << "Błąd typu 'kierunek'\n";
    
    	if(!prowizja || prowizja >= 1) cout << "Niepoprawna wartość prowizji!";
    	else this->prowizja = prowizja;
    }
    
    papier::papier(const papier &akcja)
    {
    	this->nazwa = new char [20];
    	::strcpy(this->nazwa, akcja.nazwa);
    
    	this->cenaKup = akcja.cenaKup;
    	this->cenaSprz = akcja.cenaSprz;
    	this->ilosc = akcja.ilosc;
    	this->prowizja = akcja.prowizja;
    }
    
    papier & papier::operator =(const papier &akcja)
    {
    	if(this != &akcja) 		//if object's not the same: {A = A}->only returns pointer
    	{
    		delete this->nazwa;
    
    		this->nazwa = new char [20];
    
    		::strcpy(this->nazwa, akcja.nazwa);
    
    		this->cenaKup = akcja.cenaKup;
    		this->cenaSprz = akcja.cenaSprz;
    		this->ilosc = akcja.ilosc;
    		this->prowizja = akcja.prowizja;
    	}
    
    	return *this;
    }
    
    papier::~papier()
    {
    	delete [] nazwa;
    	nazwa = NULL;
    }
    //other definitions
    //...
    If it's (as you mentioned before) hard to tell what's wrong in here, leave it at that
    I appreciate your help and concern

    ps: I followed the suggestion, and expanded the array every 10 elements added. It works
    Last edited by fx69; 02-25-2010 at 04:49 AM. Reason: p.s.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Array of struct pointers - Losing my mind
    By drucillica in forum C Programming
    Replies: 5
    Last Post: 11-12-2005, 10:50 PM
  2. Request for comments
    By Prelude in forum A Brief History of Cprogramming.com
    Replies: 15
    Last Post: 01-02-2004, 09:33 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21