Operator Overloading vs objects on heap
Author
I''m having trouble writing a class that looks a bit like a basic string but acts on unsigned char data instead. My problem however might be in the way I''m trying to use my classes. I need to always place these on the heap as they will need to be passed around within the app. I think however that my syntax might be a little wrong for using the operators with pointers...
for example :
[source
#include "iostream.h"
#include "byte.h"
void main(void)
{
ByteArray *Message1 = new ByteArray();
ByteArray *Message2 = new ByteArray("there ");
ByteArray *Message3 = new ByteArray();
ByteArray *Message4 = new ByteArray();
Message1->insert(Message1->length(),"hello ");
Message4 = "world!"; //Generates "cannot convert from ''char [7]'' to ''class ByteArray *''"
Message3 = Message1 + Message2; //Generates "cannot add two pointers"
delete Message2;
cout << Message3;
}
[/source]
These errors make sence considering the fact that I''m using pointers. Since I''ll never need to modify the pointer is there a way use references for this?
Thanks agian
gimp
How are you doing your operator overload? It might help if we could see the class 
you need to use references to get the polymorphic behaviour that you are after. Post up some more code and I will see if I can help you... You need to physically write the function that converts the "world" into something that your bytearray can understand.
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
-Chris Bennett of Dwarfsoft
"The Philosophers' Stone of Programming Alchemy"
IOL (The list formerly known as NPCAI ) - A GDNet production
Our Doc - The future of RPGs
Thanks to all the goblins over in our little Game Design Corner niche
Edited by - dwarfsoft on August 27, 2000 8:57:12 PM
you need to use references to get the polymorphic behaviour that you are after. Post up some more code and I will see if I can help you... You need to physically write the function that converts the "world" into something that your bytearray can understand.
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
-Chris Bennett of Dwarfsoft
"The Philosophers' Stone of Programming Alchemy"
IOL (The list formerly known as NPCAI ) - A GDNet production
Our Doc - The future of RPGs
Thanks to all the goblins over in our little Game Design Corner niche
Edited by - dwarfsoft on August 27, 2000 8:57:12 PM
OK, go out right now and get Scott Meyer''s book, "Effective C++". He goes through exactly how to write an efficient string class.
There are too many problems here to enumerate, but here are some:
1) If you''ve written overloads of the operators you''re trying to use, there''s no way to get at them using a pointer. Try these instead:
*Message4 = "world!";
*Message3 = *Message1 + *Message2;
cout << *Message3;
2) Dynamically allocating everything completely defeats the purpose of encapsulating a string class.
3) Considering how buggy this code is, I can''t imagine your operator overloads, even if they exist, are written correctly.
4) Why aren''t you deleting everything else you allocate? If you''ve tried to design your string class so that operator = and operator + transfer ownership of the allocated memory, you''re in for a world of hurt.
Again, buy that book and read about the right way to do things.
There are too many problems here to enumerate, but here are some:
1) If you''ve written overloads of the operators you''re trying to use, there''s no way to get at them using a pointer. Try these instead:
*Message4 = "world!";
*Message3 = *Message1 + *Message2;
cout << *Message3;
2) Dynamically allocating everything completely defeats the purpose of encapsulating a string class.
3) Considering how buggy this code is, I can''t imagine your operator overloads, even if they exist, are written correctly.
4) Why aren''t you deleting everything else you allocate? If you''ve tried to design your string class so that operator = and operator + transfer ownership of the allocated memory, you''re in for a world of hurt.
Again, buy that book and read about the right way to do things.
Author
Thanks for the advise... the * in fron of the pointer names is what I''d forgotten about...
Answering point 2. I''m not dynamically allocating everything. Here is how it works..
I have a event\messaging subsystem. Keypresses, net messages, mouse , joy, files loaded etc all place messages on a queue. The queue is read by a pluggable factory, at which point a ''handler'' function for that particular message type is called and passed the memory. The handler function deletes the message.
In this project the data that the message sits in is the ByteArray class I''m developing. The class manages it''s own buffers.
I''m aware of the dangers of having ram deleted and new''ed in different location but am prepared for that.
The ByteArray class acts like a vector but is used in a more string like manner. The messages that are passed to it to be appended can be either character based or binary(structs).
oh and the buggy code you saw was just a few hacked together lines to test the class. In these little tests I don''t bother deleting the ram I new. Following is my string like class....
thanks...
Oh and as for books... I''ll think about buying that book but I''ve bought about a load of books in the past few months ranging from building 3d engines, opengl redbook, 3d geometry, efficient c++, data structures and algorithms, photoshop techniques, photorealistic modelling, c++ in 21 days(JLiberty), Game Design, a few lamothe''s and I have 2 inches of print outs from gamasutra, gamedev, flipcode and the algorithms mailing list, mostly on terrain LOD and occlusion culling.... (Not to mention my MCSE is due for renewal)
My brain is melting....
gimp
Answering point 2. I''m not dynamically allocating everything. Here is how it works..
I have a event\messaging subsystem. Keypresses, net messages, mouse , joy, files loaded etc all place messages on a queue. The queue is read by a pluggable factory, at which point a ''handler'' function for that particular message type is called and passed the memory. The handler function deletes the message.
In this project the data that the message sits in is the ByteArray class I''m developing. The class manages it''s own buffers.
I''m aware of the dangers of having ram deleted and new''ed in different location but am prepared for that.
The ByteArray class acts like a vector but is used in a more string like manner. The messages that are passed to it to be appended can be either character based or binary(structs).
oh and the buggy code you saw was just a few hacked together lines to test the class. In these little tests I don''t bother deleting the ram I new. Following is my string like class.... #include "assert.h"#include "string.h"#include "string"/*Still to implement bool operator != (ByteArray &, ByteArray &);*/class ByteArray{public: inline ByteArray() { buffer=0; resize (0); } inline ByteArray(char *cp) { buffer = 0; resize (strlen(cp)); strcpy ((char *)buffer, cp); } inline ByteArray(unsigned char *cp,unsigned long size) { buffer = 0; resize (size); memcpy ((unsigned char *)buffer, (unsigned char *)cp,size); } inline ByteArray(ByteArray &ba) { buffer = 0; resize (ba.length()); memcpy (buffer, ba.buffer, ba.bufferLength+1); } inline ~ByteArray() { delete [ ] buffer; } inline int length () { for (unsigned long i = 0; i < bufferLength; i++) if (buffer<i> == ''\0'') return i; return bufferLength; } inline void resize(unsigned long newLength) { // if no current buffer, length is zero if (buffer == 0) bufferLength = 0; // case 1, getting smaller if (newLength < bufferLength) { // just add new null character buffer[newLength] = ''\0''; } else { // case 2, getting larger // allocate new buffer, allow space for null character unsigned long i; unsigned char * newbuffer = new unsigned char[newLength + 1]; assert (newbuffer != 0); // first copy existing characters for (i = 0; i < bufferLength && buffer[i] != ''\0''; i++) newbuffer[i] = buffer[i]; // then add pad characters memset(newbuffer+i,0,newLength); // add terminating null character newbuffer[i] = ''\0''; // free up old area, assign new if (buffer != 0) delete [ ] buffer; buffer = newbuffer; bufferLength = newLength; } } inline void insert (unsigned long position, const char *newText) { unsigned long len = length(); unsigned long ntLen = strlen(newText); unsigned long newLen = len + ntLen; // if necessary, resize buffer resize(newLen); // move existing characters over for (unsigned long i = len; i > position; i--) buffer[i + ntLen] = buffer[i]; // insert new characters for (unsigned long j = 0; j < ntLen; j++) buffer[position + j] = newText[j]; } inline bool empty () { return buffer[0] == ''\0''; } inline void operator += (const char *data) { insert (length(), data); } inline void operator += (ByteArray & right) { insert (length(), (const char *)right.buffer); } inline unsigned char & operator [ ] (unsigned int index) { assert (index <= bufferLength); // not required by standard return buffer[index]; } inline void operator = (ByteArray &ba) { resize (ba.length()); strcpy ((char*)buffer, (char*)ba.buffer); } inline int operator == (ByteArray &ba) { if (bufferLength = ba.bufferLength) return memcmp(buffer,ba.buffer,bufferLength); //This is wrong... else return false; } inline ByteArray operator + (ByteArray &ba) { ByteArray clone(*this); // copy left argument clone += ba; // append right argument return clone; // return result }protected: unsigned char *buffer; unsigned long bufferLength;}; thanks...
Oh and as for books... I''ll think about buying that book but I''ve bought about a load of books in the past few months ranging from building 3d engines, opengl redbook, 3d geometry, efficient c++, data structures and algorithms, photoshop techniques, photorealistic modelling, c++ in 21 days(JLiberty), Game Design, a few lamothe''s and I have 2 inches of print outs from gamasutra, gamedev, flipcode and the algorithms mailing list, mostly on terrain LOD and occlusion culling.... (Not to mention my MCSE is due for renewal)
My brain is melting....
gimp
I must admit I haven''t read every post yet, and I know it''s a lame answer, but inline''ing that big functions (resize, insert) could even slow down your code and surely will provide a uge exe file...
Well, it's not as bad as I'd feared 
, but there are still some problems:
- operator == should return bool and should be a const function.
- operator + should also be const, and should return a ByteArray& to the temporary object so you can chain concatenations (a = b + c + d).
- operator += should return a ByteArray& to *this (but can't be const like operator +).
I'm not sure, but since you're not doing anything different with operator += than a = a + b (i.e., you're not optimizing it), I think you can leave it out. If you define operator + and operator = correctly, I think the compiler will substitute those two for operator +=. Actually, you do save one function call by doing it your way, so, it's up to you.
It looks like your resize command will run past the end of your buffer on the memset line. For instance, let's say bufferLength is 5 and you want a new length of 10. After the for loop, i will be 5. Then you memset from newBuf + 5 for 10 chars, but you've only allocated 10 chars total to the newBuf. You've overrun by 5--bad things will happen.
If you're always treating your string like a zero-terminated string anyway, there's no need to clear all the data past the end of the string in the first place--a simple null character will do. This way, instead of doing the hand-copying and memset stuff you're doing now, your resize routine can just allocate new data and then do a strcpy (newBuf, oldBuf).
I'd suggest making resize a private/protected function. No need for the public to be resizing your internal buffers.
As a final suggestion, it's really handy to have this:
operator const char* ByteArray () const
{
return buffer;
}
This will allow you to pass your string into a plethora of standard C and C++ library functions as a const char string, including cout, printf, etc.
Hold up--I just noticed that you're working with unsigned char instead of signed char (yeah, I know, the first sentence of your post). Therefore I don't know if the operator const char* or the strcpy stuff will work, but shouldn't it? Anyway, good luck. Most of my advice is still applicable.
Edited by - Stoffel on August 28, 2000 11:38:37 AM
, but there are still some problems:- operator == should return bool and should be a const function.
- operator + should also be const, and should return a ByteArray& to the temporary object so you can chain concatenations (a = b + c + d).
- operator += should return a ByteArray& to *this (but can't be const like operator +).
I'm not sure, but since you're not doing anything different with operator += than a = a + b (i.e., you're not optimizing it), I think you can leave it out. If you define operator + and operator = correctly, I think the compiler will substitute those two for operator +=. Actually, you do save one function call by doing it your way, so, it's up to you.
It looks like your resize command will run past the end of your buffer on the memset line. For instance, let's say bufferLength is 5 and you want a new length of 10. After the for loop, i will be 5. Then you memset from newBuf + 5 for 10 chars, but you've only allocated 10 chars total to the newBuf. You've overrun by 5--bad things will happen.
If you're always treating your string like a zero-terminated string anyway, there's no need to clear all the data past the end of the string in the first place--a simple null character will do. This way, instead of doing the hand-copying and memset stuff you're doing now, your resize routine can just allocate new data and then do a strcpy (newBuf, oldBuf).
I'd suggest making resize a private/protected function. No need for the public to be resizing your internal buffers.
As a final suggestion, it's really handy to have this:
operator const char* ByteArray () const
{
return buffer;
}
This will allow you to pass your string into a plethora of standard C and C++ library functions as a const char string, including cout, printf, etc.
Hold up--I just noticed that you're working with unsigned char instead of signed char (yeah, I know, the first sentence of your post). Therefore I don't know if the operator const char* or the strcpy stuff will work, but shouldn't it? Anyway, good luck. Most of my advice is still applicable.
Edited by - Stoffel on August 28, 2000 11:38:37 AM
That''s just what I was thinking, but the board ate your code!
Just use "mystringtype" as a STL string. If you''re wondering, the STL string class is defined as:
So there is really no magic going on behind the scenes - just templates and typedefs.
- null_pointer
Sabre Multimedia
typedef basic_string<unsigned char> mystringtype;
Just use "mystringtype" as a STL string. If you''re wondering, the STL string class is defined as:
typedef basic_string<char> string;
So there is really no magic going on behind the scenes - just templates and typedefs.
- null_pointer
Sabre Multimedia
Man, you crazy? What made you inline EVERYTHING?! Constructors, destructors... damn... you want everything working fast as hell #;oD
this is the first and last function i looked at and here is what i have to say about it...
inline int operator == (ByteArray &ba)
{
if (bufferLength = ba.bufferLength)
return memcmp(buffer,ba.buffer,bufferLength); //This is wrong...
else
return false;
}
bufferLength = ba.bufferLength is NOT the same as
bufferLength == ba.bufferLength and what you want is the latter...
memcmp should be !memcmp... and instead of having and if/else you could do this, instead of inlining everything thinking it would be faster.. it''s all in the algorithm you''re using.
inline int operator == (ByteArray &ba)
{
return (bufferLength == ba.bufferLength && !memcmp(buffer,ba.buffer,bufferLength);
}
without having looked at the rest of your code, this is what I would''ve done if I were you #:oP
Take care... and look at your other code for mistakes like the above. And for god... God''s sakes... stop inlining everything #;oD
-------------------------------
That's just my 200 bucks' worth!
..-=gLaDiAtOr=-..
this is the first and last function i looked at and here is what i have to say about it...
inline int operator == (ByteArray &ba)
{
if (bufferLength = ba.bufferLength)
return memcmp(buffer,ba.buffer,bufferLength); //This is wrong...
else
return false;
}
bufferLength = ba.bufferLength is NOT the same as
bufferLength == ba.bufferLength and what you want is the latter...
memcmp should be !memcmp... and instead of having and if/else you could do this, instead of inlining everything thinking it would be faster.. it''s all in the algorithm you''re using.
inline int operator == (ByteArray &ba)
{
return (bufferLength == ba.bufferLength && !memcmp(buffer,ba.buffer,bufferLength);
}
without having looked at the rest of your code, this is what I would''ve done if I were you #:oP
Take care... and look at your other code for mistakes like the above. And for god... God''s sakes... stop inlining everything #;oD
-------------------------------
That's just my 200 bucks' worth!
..-=gLaDiAtOr=-..
Author
Thanks guys for the help... I have it mostly up and running now. I have one last overload to add : []
I''ve tried some simple tests but still can get sample code like the following to work:
*Message[0] = ''h'';
I think I might be missing the point again. Does this kind of assignment need both the [] and = operators, with the assignment overloaded to accept const char *? Or is there another way?
One last question. If I wanted to do the following :
*message[0] = MSG_STRINGCOMMAND;
Where MSG_STRINGCOMMAND is an enum that I know is less than 256 should I just cast it to unsigned char or is there a better way? Or further to that if I wanted to copy a 6 byte struct in to bytes 2-7 how should my class handle it?
Thanks again..
gimp
PS : NULL, I guess I''m at another one of those points where I want to do it myself mostly so I know HOW to do this stuff. I''ve been avoiding overloads and operators for way too long.
I''ve tried some simple tests but still can get sample code like the following to work:
*Message[0] = ''h'';
I think I might be missing the point again. Does this kind of assignment need both the [] and = operators, with the assignment overloaded to accept const char *? Or is there another way?
One last question. If I wanted to do the following :
*message[0] = MSG_STRINGCOMMAND;
Where MSG_STRINGCOMMAND is an enum that I know is less than 256 should I just cast it to unsigned char or is there a better way? Or further to that if I wanted to copy a 6 byte struct in to bytes 2-7 how should my class handle it?
Thanks again..
gimp
PS : NULL, I guess I''m at another one of those points where I want to do it myself mostly so I know HOW to do this stuff. I''ve been avoiding overloads and operators for way too long.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
