CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 6 of 6

Thread: String problem

  1. #1
    Join Date
    Apr 2010
    Posts
    2

    String problem

    I had to program a string class as exercise. I should provide a method concat.

    Whenever I use it the program stops working without showing some error or something else. It just quits and does not process the following lines after I use the concat method.
    I played around and figured out that when I change the signature to
    MyString& concat(MyString& my);

    it works but by copy it does not with the following:
    MyString& concat(MyString my);

    I don't understand that.

    Code:
    My header file:
    #ifndef MYSTRING_H_
    #define MYSTRING_H_
    
    class MyString {
    private:
    	char* characters;
    	int size;
    
    	int length(char* c);
    public:
    	MyString();
    	MyString(char* c);
    	MyString(const MyString& my);
    
    	virtual ~MyString();
    
    	char* toCString();
    
    	MyString& concat(MyString my);
    
    	int getLength();
    };
    
    #endif /* MYSTRING_H_ */
    
    
    MyString class:
    
    #include <iostream>
    #include "MyString.h"
    
    MyString::MyString() {
    	//Init char with size 0
    	characters = new char[1];
    	size = 0;
    	characters[0] = '\0';
    }
    
    MyString::MyString(const MyString& my) {
    	characters = my.characters;
    	size = my.size;
    }
    
    int MyString::getLength() {
    	return size;
    }
    
    MyString::MyString(char* c) {
    	characters = c;
    	size = length(c);
    }
    
    char* MyString::toCString() {
    	return characters;
    }
    
    int MyString::length(char* c) {
    	int i = 0;
    	while(c[i] != '\0') {
    		i++;
    	}
    
    	return i++;
    }
    
    
    MyString& MyString::concat(MyString my) {
    	char* c = my.toCString();
    
    	int l1 = getLength();
    	int l2 = length(c);
    
    	size = l1 + l2;
    
    	char* temp = new char[size+1];	//+1 for \0
    
    
    
    	for(int i = 0; i < l1; i++) {
    		temp[i] = characters[i];
    	}
    	for(int i = 0; i < l2; i++) {
    		temp[l1+i] = c[i];
    	}
    	temp[size] = '\0';	//Termination
    
    
    	//Copy string
    	characters = temp;
    
    
    	return *this;
    }
    
    
    
    MyString::~MyString() {
    	//free memory
    	delete[](characters);
    }
    Last edited by cilu; April 7th, 2010 at 09:32 AM. Reason: code tags

  2. #2
    Lindley is offline Elite Member Power Poster
    Join Date
    Oct 2007
    Location
    Seattle, WA
    Posts
    10,895

    Re: String problem

    Your copy constructor is incorrect. Right now, once you use it you'll have two MyString objects pointing at the same physical character array in memory. One one them is destroyed, that array gets deleted......leaving the second one pointing to invalid memory.

    Also, you need to define operator=.

  3. #3
    Join Date
    Oct 2002
    Location
    Timisoara, Romania
    Posts
    14,360

    Re: String problem

    Here are some things: why do you have a length() function that takes a char* and returns its length? Shouldn't it refer to the current object? So is shouldn't have any parameters. Moreover, why don't you use strlen() to return the length? And even more, you already know the length from your variable size. You only have to return that value.

    Then, in concat() you keep allocing new memory, but you never release the old memory. You should do that just before this assignment:
    Code:
    characters = temp;
    Now, the real problem is this: whenever you handle memory allocation manually in a class (i.e. you have pointers) you must implement a copy constructor and an assignment operator and do a deep copy, as oppose to the implicit shallow copy that only copies the values of the variables and not the object they point to. You have implemented a copy constructor:
    Code:
    MyString::MyString(const MyString& my) {
    	characters = my.characters;
    	size = my.size;
    }
    But all you do there is a shallow copy: you copy the value of the pointer (i.e. the address it points to), not the object that it points to. When you destroy the string, the first string will release the memory and the second will crash when attempting to destroy something that was already destroyed.

    Here is how a deep copy constructor should look:
    Code:
    MyString::MyString(const MyString& my) {
      size = my.size; 
      characters = new char[size+1];
      strcpy(characters, my.characters);
    }
    I suggest you reimplement this constructor MyString(char* c) in the same manner, and an assignment operator. Then you can fix the concat function too.
    Last edited by cilu; April 7th, 2010 at 10:51 AM.
    Marius Bancila
    Home Page
    My CodeGuru articles

    I do not offer technical support via PM or e-mail. Please use vbBulletin codes.

  4. #4
    Lindley is offline Elite Member Power Poster
    Join Date
    Oct 2007
    Location
    Seattle, WA
    Posts
    10,895

    Re: String problem

    There's a destructor there.

  5. #5
    Join Date
    Oct 2002
    Location
    Timisoara, Romania
    Posts
    14,360

    Re: String problem

    Quote Originally Posted by Lindley View Post
    There's a destructor there.
    Oops, the scroll didn't work well. . I have adjusted my post.
    Marius Bancila
    Home Page
    My CodeGuru articles

    I do not offer technical support via PM or e-mail. Please use vbBulletin codes.

  6. #6
    Join Date
    Apr 2010
    Posts
    2

    Re: String problem

    @Lindley
    Thanks
    @cilu
    Thanks a lot for your explanations! I made the changes and it works now. I was not supposed to use existing string functions, that's why I didn't use strlen and so on.
    The getLength method refers to the current object. The private one is needed in the concat method, since I need it for the MyString object from the parameter and cannot use strlen.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured