Allocate Memory

I'm writing a String class, but I find I'm having trouble allocating memory. It contains two member variables, m_Length (int; the length of the string, not including the null at the end) and m_Char (* char to the contained string)

Upon debugging, I find that the same memory location is set to m_Char everytime for every instance of the class. Obviously this is a huge problem because every string I create points to the same string.

This is the function I created to allocate memory

void zString::Alloc(const unsigned int & Bytes)
{
if (m_Length) delete [] m_Char;
if (Bytes)
{
m_Char = new char[Bytes+1];
for(int i=0; i<=Bytes + 1; i++)
m_Char[ i ] = '\0';
}
m_Length = Bytes;
}

By the way, I like the look of this forum

Is this for school? If not, there is already a string class in C++, std::string.

Regards,

Paul McKenzie

Also, please post your entire class and the program you are running to test it. Post only a small main() program that duplicates the problem.

Did you write a copy constructor and assignment operator for your string class? This is probably where your problem is.

Also, if you don't have to do this, again, use std::string. It is already written, so there is no need to reinvent the wheel.

Regards,

Paul McKenzie

Allright.

Well, I might use the std::string class. Where can I learn how to use it?

Even if I decide to do that, I'd like to know what I did wrong. (I did write a copy constructor)

also, I'm using Dev-cpp if it matters

Quote:

---

Originally Posted by msg555

Allright.

Well, I might use the std::string class. Where can I learn how to use it?

---

Practically every C++ book discusses usage of std::string. If the book is good, it should be in chapter 1 or 2.

I will look at your code later.

Regards,

Paul McKenzie

One question:

Why aren't you using, at the very least, the basic <string.h> functions in your class or even functions such as memcpy() when copying one buffer to another?

For example:

Code:

---

zString::zString(const char * const rhs)
{
    for(m_Length=0; *(rhs+m_Length)!='\0'; m_Length++) {} //Find first Null
    Alloc(m_Length);
    for (int i=0; i<m_Length; i++)
        (*this)[i] = *(rhs+i);
}

---

There is no need to code loops to find the length of a null-terminated string, or loops to copy data.

Code:

---

zString::zString(const char * const rhs)
{
    m_Length = strlen(rhs);
    Alloc(m_Length);
    memcpy(m_Char, rhs, m_Length);
}

---

Regards,

Paul McKenzie

I can only answer your question with a simple, "because I feel like it"
Is there any real advantage to coding it that way?

Also, I read "Sams teach yourself c++" book. It nevered discusses std::string, although it go over some of the function in string.h like memcpy

Quote:

---

Originally Posted by msg555

I can only answer your question with a simple, "because I feel like it"

---

That is not the way to approach programming in C++. We will give you answers based on the experience that we have here, which is not based on "feel" but on what works and is used by professionals. When another C++ programmer looks at your code, they will also wonder why you didn't use the library routines. It isn't just myself that will say this.

Quote:

---

Is there any real advantage to coding it that way?

---

If you are talking about using the standard library, yes. It is already written for you, it is standard, it is as efficient as possible, it has no bugs, and other C++ programmers can understand the code much more quickly, since the functions used are standard. If I took a quick glance, I have no idea if those loops you wrote are correct. However, if I saw a simple call to strlen() or memcpy(), then this is fully understandable to myself and other programmers what those functions do.

Quote:

---

Also, I read "Sams teach yourself c++" book. It nevered discusses std::string,

---

Throw the book away. It is either outdated (pre 1998), or it is a piece of junk. It isn't anything other than those two options.

Regards,

Paul McKenzie

There are many things wrong with your class. Here are three of them:

1) You should initialize the m_Char to NULL on construction of the zString object.

2) Your assignment operator does not return a value

3) Why does your destructor call Alloc()? Why not just call delete []?

Code:

---

#include <string.h>

zString::~zString()
{
    delete [] m_Char;
}


zString::zString(const char * const rhs)
  : m_Char(0)  // Do this for each constructor
{
    m_Length = strlen(rhs);
    Alloc(m_Length);
    memcpy(m_Char, rhs, m_Length);
}

bool zString::operator==(const zString & rhs) const
{
    return (strcmp(m_Char, rhs.m_Char) == 0);
}

zString & zString::operator= (const zString & rhs)
{
  Alloc(rhs.Length());
  strcpy( m_Char, rhs.m_Char);
  return *this;
}

void zString::Alloc(const unsigned int & Bytes)
{
    delete [] m_Char;
    m_Char = new char[Bytes+1];
    memset(m_Char, 0, Bytes+1);
    m_Length = Bytes;
}

---

Just these changes alone is all that I needed to get this to work. But I would never use this, since again, std::string is there so that you don't need any of this code.

Regards,

Paul McKenzie

Thanks for you help Paul.

Quote:

---

You should initialize the m_Char to NULL on construction of the zString object.

---

I'll be sure to add that.

Quote:

---

Your assignment operator does not return a value

---

lol, that is going to be a problem.

Allright, so the problem is caused by not setting m_Char to NULL. I don't get why that caused a problem considering every constructor calls Alloc and Alloc set's m_Char to a new block of memory.

Quote:

---

Originally Posted by msg555

Allright, so the problem is caused by not setting m_Char to NULL. I don't get why that caused a problem considering every constructor calls Alloc and Alloc set's m_Char to a new block of memory.

---

Please look carefully at your Alloc code, and see what I changed it to. The very first line in Alloc is in error:

Code:

---

if (m_Length)
    delete [] m_Char;

---

You set m_Length to a value, and then you are attempting to call delete on an invalid pointer. m_Char has a garbage value when the constructor was called, and calling delete [] on this value leads to unpredictable results. That is why setting it to NULL in the member-initialization list ensures that calling delete will not have any effect (calling delete on a NULL pointer is valid).

This is why std::string was developed, so you don't need to write code like this and cause bugs to occur.

As to books:

http://www.accu.org/bookreviews/public/index.htm

"C++ Primer Plus" - Lippman
"Accelerated C++" - Koenig & Moo
"The C++ Standard Library" - Josuttis
"The C++ Programming Language 3rd edition" - Stroustrup

Regards,

Paul McKenzie