• Advertisement

Archived

This topic is now archived and is closed to further replies.

Operator Overloading vs objects on heap

This topic is 6350 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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

Share this post


Link to post
Share on other sites
Advertisement
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

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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....

    
#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

Share this post


Link to post
Share on other sites
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...

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
What don''t you just do


#define basic_string Unsigned_String


Unsigned_String My_String!!

Share this post


Link to post
Share on other sites
That''s just what I was thinking, but the board ate your code!


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

Share this post


Link to post
Share on other sites
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=-..

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
hehe I guess I''ll stop suggesting ideas...but you have a big topic to learn

Anyway, what you do to overload the [] operator is simple. Just check out the following very simple class, which looks and acts like a string. I think this is what you are looking for:


class string
{
public:
inline string(const unsigned int size);
inline virtual ~string();

private:
unsigned char* elements;
};

inline string::string(const unsigned int size)
: elements(NULL) { elements = new unsigned char[size]; }

inline string::~string()
{ delete[] elements; }



Now there are two methods to using the [] with the string class.


METHOD #1

Here''s the first, as it is much simpler to understand:


// inside the class definition:
inline unsigned char& operator [] (const unsigned int index);
inline const unsigned char& operator [] (const unsigned int index) const;

// outside the class definition:
inline unsigned char& string::operator [] (const unsigned int index)
{ return( elements[index] ); }

inline const unsigned char& string::operator [] (const unsigned int index) const
{ return( elements[index] ); }



There are two versions, one when you are just reading the value of the element, and one when you are writing the value of the element. The compiler should pick one unambiguously. Don''t worry about all of the const stuff yet, just focus on the parameter and the return value. The parameter is unsigned, which means we don''t want any negative numbers. The return value is a reference, so that the user of the class can change the actual element and not a temporary copy. The operator functions are inlined to eliminate any overhead and possible temporary objects.

Basically, what the functions do is simply look up the requested element and return it. No magic there.


METHOD #2

Conversion is the hard part of programming classes, but it makes them MUCH more flexible...

There are two kinds of conversion when dealing with classes. One kind uses constructors that have one and only one argument, while the other kind uses special conversion operators. Conversion operators are nifty things that allow you to make some very interesting code. I''ll get into that in a minute. Remember that little string class? We''ll add both types of conversion to it, and the string class should be extremely flexible.


2.a Constructor conversion

This is interesting... Anyway, constructor conversion occurs when you pass an object that the compiler wasn''t expecting, so it looks for a way to convert the object that you passed, into another object. In this case, we want to convert from const char* (like a hard-coded string: "hello world!") into our own array object. To let the compiler see how to do this, we add another constructor to the array class, which duplicates the const char array.


// add to class definition:
string(const unsigned char* elements);

// add outside of class definition:
string::string(const unsigned char* elements)
: elements(NULL)
{
this->elements = new unsigned char[strlen(elements)];
strcpy(this->elements, elements); // copy it
}



Now that the code is in place, let''s look at what we can do with that constructor.


void some_function(string some_string)
{
}

int main(int, char*)
{
some_function("hello world!"); // converted to an string object automatically!!
}



To explain what the constructor is for, we are telling the compiler to construct one object from another. We are telling it how to build an array object from an unsigned char*, kind of like a blueprint.


2.b Special conversion operators

(First, remove or comment out the [] operators if you have added them!)

This is the neat part! Now, we have told the compiler how to convert an unsigned const char* to our string class, but we haven''t told the compiler how to convert our string class to an unsigned const char*. To do that, we can use the special conversion operators. Here are the ones we need:


// add to the class definition:
inline operator unsigned char* ();
inline operator const unsigned char* () const;

// add outside the class definition:
inline string::operator unsigned char* ()
{ return( elements ); }

inline string::operator const unsigned char* () const
{ return( elements ); }



Simple, huh?

Now, when you do this:


array myarray("hello world!"); // use our conversion constructor!

for( int x=0; x < 11; x++ )
{
// just set it to 0
myarray[x] = 0; // use our conversion operator!
}



When the compiler hits the [], it checks for an operator that allows it to be treated like an regular array. In this case, since an unsigned char array (like "char myarray[256]") is really just a char*, the conversion works fine. All array identifiers are really only pointers to the first element...

Think about it for a while... Arrays are stored contiguously, and all elements have the same size. So, when you do this:


myarray[x] = 0;



The compiler sees this:


*(myarray + x) = 0;



Just good ol'' pointer math. That little statement just says that we want the xth element from the beginning of the array. Let''s say that myarray''s operator char* returns the address:

0x40000015

That is where the first element is stored. Now, if ''x'' from our loop example happens to be 2 right now, we get:

0x40000015 + 0x00000002 = 0x40000017

Which means that the third element in the array is stored at the address 0x40000017 in memory. The fourth element is stored at 0x40000018 in memory, and so on...

There''s just no way to program without knowing pointers.

Anyway, if I''m not clear enough, just post a question or step through this code in a console app to see which functions/operators are called at what times. If there are any errors with this class, just post it here and either myself or someone else can clear this up. I wrote a string class a while back, so most of this is from memory...

I always say, if you can write a full-fledged string class, you can do just about anything else with the language, because it has such a mix of all the neat stuff in it.



- null_pointer
Sabre Multimedia

Share this post


Link to post
Share on other sites
ECKiler: because he''s writing the string class to learn design, not to get a working string class.

null: Wow, you learn something new every day. I didn''t know that overloading operator char* would let you use array indexing (though it makes sense now that I think about it).

A couple of style points:
In all of my classes, I have every member variable preceded by a "m_". Yes, stolen from MFC. But this accomplishes two things: First, you you have a very long member function, it''s easy to tell which variables are class members (they have m_) and which are function-local (they don''t). Second, it keeps you from argument tomfoolery, as in the example conversion constructor; if you''d named you member variable "m_elements" instead of "elements", you wouldn''t have had the funky this->elements call.

Finally, I''m all for having an operator const char*, but I''m uncomfortable with a non-const operator. I think it opens up the class too much to outside mucking around. Just MHO, though.

Share this post


Link to post
Share on other sites
Instead of passing around pointers all over the place, the de-referencing them when you use them. Maybe you could use a reference instead (providing it can never be NULL). Then you''ll be able to use normal syntax, "Message4 = "world!";" instead of "*Message4 = "world!";".


quote:

Stoffel:
operator + should also be const, and should return a ByteArray& to the temporary object



Are you sure you meant to say that? Return a reference to a temporary is always a bad idea, operator + should always return by value. If you do use a reference, when the function returns it''s be referring to an object that doesn''t exist anymore (it''s been popped of the stack).

quote:

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.



Nope, it wont. Using += is much better than X = X + Y because it''s more efficient, no temporary is created or returned. So it''s better to implement operator + using operator +=.

quote:

As a final suggestion, it''s really handy to have this:
operator const char* ByteArray () const
{
return buffer;
}



I''d probably go for a GetChar function that returns a const char*, most of the time it''s better to be explicit when you want something to be converted. It helps avoid problems with conversions you didn''t really want to happen. That''s also why it''s a good idea to make your constructors explicit if they can be called with one parameter (unless it really is a conversion or copy constructor).

Share this post


Link to post
Share on other sites
To add to Wilka''s suggestion, you can use references with heap memory, but you have to make sure you call delete!


int main(int, char*)
{
int& myint = *(new int); // choose your own syntax!

myint = 10; // no problem here!
myint = 5;
myint = 43789;

delete(&myint); // delete! delete!
}



Stoffel:

Yeah, I picked that up when working on my matrix class - you can actually make working [][] operators! Check this out:


template <unsigned int rows, unsigned int columns, typename element_type = float>
class matrix
{
public:
element_type* operator[](unsigned int index) { return( &array[index][0] ); }

private:
element_type elements[rows][columns];
};



Which allows you to do this:


int main(int, char*)
{
matrix<4, 4> the_matrix;

the_matrix[0][0] = 0.50f;
the_matrix[2][3] = 0.50f;
}



Even though that code doesn''t do anything particularly useful, you can see it uses the operators!


BTW, that non-const operator is not as bad as you may think. This is harmless:


string mystring("hello world!");
mystring = (unsigned char*) NULL;



Because mystring::operator unsigned char* () returns a temporary pointer, not a reference! However, you could delete the pointer from outside the class, which would make the string destructor crash whenever it is called...


Actually, I think the this->member style is much more readable than the m_member style. Which do you say normally, 1 or 2?

1. This keyboard is cleaner than that keyboard.
2. m_keyboard is cleaner than that keyboard.

I dislike prefixes... wDo wYou wTalk cmpLike pThis nAll m_Of adrThe tiTime? pI lkMean, pThis prBecomes stTiring tiAfter wA tiWhile...



- null_pointer
Sabre Multimedia

Share this post


Link to post
Share on other sites
quote:

int main(int, char*)
{
int& myint = *(new int); // choose your own syntax!

myint = 10; // no problem here!
myint = 5;
myint = 43789;

delete(&myint); // delete! delete!
}



ick! I'm not to keen on that Deleting the address a variable (even if it is just a reference) looks nasty. I'd prefer something like this (although it does use an extra var).


int main(int, char*)
{
int* pMyInt = new int; //look! a prefix
int& myInt = *pMyInt;

myint = 10; // no problem here!
myint = 5;
myint = 43789;

delete pMyInt; // delete! delete!
}


There's not much of an advantage in uses a reference like this (but it does serve as an example ).

You'd probably use them more like this:


int main(int, char*)
{
int* pMyInt = new int;

Function_That_Takes_An_Int_By_Ref(*pMyInt);

delete pMyInt; // delete! delete!
}


Edited by - Wilka on August 30, 2000 5:44:59 PM

Share this post


Link to post
Share on other sites
Whoa!!!
quote:

Stoffel:
operator + should also be const, and should return a ByteArray& to the temporary object
Wilka:
Are you sure you meant to say that?


Holy smokes, hell no! Disregard what I said! Thanks Wilka. Guess I'm just not on my game lately.

Returning a reference or pointer to a temporary will almost ALWAYS get you in a big batch of screwed. Don't do it. Yes, return the actual object instead. This means you have to have a tight copy constructor that works.

re: prefixes on variables (null-ptr), it's again a style/preference thing. The problem is, the projects I work on have such a huge scope that it's easy to get lost. I have many 5000-line MEMBER FUNCTIONS I have to maintain (no, I didn't write them--I just have to maintain them). In cases like that, it's nice to have the prefixes around.

I find it much less annoying to have p or sz or c or n or r in front of a variable name than the alternative: "was this a pointer? is this an index or a size?", etc. etc.

If you don't like prefixes, then I'd rename the variable in your constructor. The this->thing just bugs the hell out of me. I'd rather do "ByteArray (const char* element_arg)", and keep the class variable name as "element".

By the way, Wilka brings up a good point: gimp, why are you putting everything on the heap (i.e. using new) in the first place? Whether or not an object is created on the stack or on the heap should make no difference in how the class works.

Edited by - Stoffel on August 30, 2000 6:33:36 PM

Share this post


Link to post
Share on other sites
Personally, I''ve never seen the need for either prefixes or using ''this->'' explicitly. My justification is that your classes, and your functions, should be small enough and encapsulated enough that there shouldn''t really be any ambiguity. The only time I have problems is with functions such as...

void SetCoords(int newX, int newY);

...where the logical names for the parameters are the same as the members they are being assignd to (in this case, x and y). So I normally call them ''newX'' or whatever.

But yeah, in general, if you don''t know what a variable is just by looking at the function, then either your function is too complex or you use too many globals

Share this post


Link to post
Share on other sites
gimp:

So...did we help fix the problems?


Regarding naming conventions:

(DISCLAIMER: this is seriously off-topic and it''s all my own opinion so just ignore me if you want to! )

5000 line functions...Stoffel, just make sure you don''t have any feelings of violence or revenge against the authors of those functions...

(BTW did you count the lines yourself? )

OK, I just deleted a long argument that wasn''t going anywhere. I''ve put it in a theory. There are three properties that objects in C++ have: name, type, and scope. The language should be designed such that none of them have to bleed into each other. So, under this theory, m_ is really just a rule to remember scope, but that information is already a property of the object (where you declare it). Also, "dw" "p" "n" etc. are just rules to remember type (how you declare it), but that too is already a property of the object...

So in normal code (functions that you can read in a few minutes), that theory works fine for me. It''s all part of interpreting code, right? Why should everyone create their own rules when the capability already exists in the language?

BTW, if you want to see something interesting, I finished that "readability.hpp" file... I''ve added a few "keywords" and the changes are really very usable and intuitive. In fact it doesn''t have any abbreviations... Here''s a sample so you can see how it looks:


class point
{
public:
inline point(constant integer x, constant integer y);

inline void operator *= (constant integer factor);

private:
integer x;
integer y;
};

inline point::point(constant integer x, constant integer y)
: x(x), y(y) {}

inline void point::operator *=(constant integer factor)
{ x *= factor; y *= factor; }

integer main(integer argument_count, character pointer arguments)
{
constant small number factor = 10;

point a(1, 5);
point b(15, 10);

a *= factor;
b *= factor;

constant decimal reduction = 0.20;

constant large integer reduced_factor = factor * reduction;

a *= reduced_factor;
b *= reduced_factor;
}



Getting the new "keywords" to show up in the neat blue color makes this code very readable - plus I''ve made the logical operators into words (or, not, and) and colored them blue as well... Just for fun, you understand!



- null_pointer
Sabre Multimedia

Share this post


Link to post
Share on other sites
gimp: don''t inline so much code. Contrary to what you might think, inlining can actually SLOW DOWN a program if you do it too much. Optimizers don''t always optimize inline code as well as they do normal code. Also, inlining library code is bad -- any time you make a change to that class anyone who uses it will be forced to recompile. I admit for templates the second doesn''t matter so much because the code all likely has to be in the header regardless, but don''t think of inlining as some kind of performance silver bullet.

Share this post


Link to post
Share on other sites
Here is a copy of how I would rewrite the class (based on what you showed). Comments are given where I changed things in main along with why, but the class header is changed (created) without significant documentation. The form is rather standard and easy to follow. The implementation has been changed (moved) to it''s own file also but I didn''t fix much of it. The others in this thread pointed out all of the other problems, I just wanted to show you the accepted way to structure a class. E-mail me any questions.

/* -- main.cpp -- */

#include // <> for std libarary classes, i doubt you wrote your own
#include // as far as i know strcmp(), etc. are in here
#include "byte.h"

void main(/*void*/) // the void isn''t necessary, C used to require but C++ doesn''t
{
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 *''"
*Message4 = ByteArray("world!");

/* the original line attempts to assign a character array to a
* ByteArray pointer, hence the cannot convert error. to access the
* object identified by Message4 the pointer must be dereferenced
* and then it may be treated as normal. with Message4 dereferenced
* the line still attempts to assign a character array to a ByteArray,
* an operator you haven''t overloaded. either overload the = operator
* to take a character array or wrap the character array in a ByteArray
* as shown in the replaced line. */

//Message3 = Message1 + Message2; //Generates "cannot add two pointers"
*Message3 = *Message1 + *Message2;

/* again the identifiers must be dereferenced to access the objects
* rather than the pointers themselves. granted pointer arithmetic
* is allowed but i''m not going to explain all that (someone else
* in this thread does). dereferencing the objects allows this line
* to operate as obviously intended. */

cout << *Message3;

/* dereference the pointer to display the object, not the identifier.
* also you still need to overload operator<< for this to display
* anything */

delete Message1;
delete Message2;
delete Message3;
delete Message4;

/* delete all of your messages, not just one. you can be careless/inefficient
* with memory management in java but not in c++. */

}

/* -- byte.h -- */

/*
Still to implement--there you go

bool operator!=(const ByteArray& ba1, const ByteArray& ba2)
{
if (strcmp(ba1, ba2) == 0 && ba1.bufferLength == ba2.bufferLength)
return true;
else
return false;
}
*/

#ifndef BYTE_ARRAY_H
#define BYTE_ARRAY_H

class ByteArray
{
public:
// constructors
ByteArray();
ByteArray(char *cp);
ByteArray(unsigned char *cp,unsigned long size);
ByteArray(ByteArray &ba);

// destructor
~ByteArray();

// accessors
int length () const;
bool empty () const {return buffer[0] == ''\0'';}

// mutators
void resize(unsigned long newLength);
void insert (unsigned long position, const char *newText);
void operator+=(const char *data);
unsigned char& operator[](unsigned int index);
void operator+=(const ByteArray& right);
void operator=(const ByteArray& ba);
int operator==(const ByteArray& ba);
ByteArray operator+(const ByteArray& ba);

protected:
unsigned char *buffer;
unsigned long bufferLength;
};

#endif // BYTE_ARRAY_H

/* -- byte.cpp --*/

#include "byte.h"

ByteArray::ByteArray()
{
buffer = 0;
resize(0);
}

ByteArray::ByteArray(char *cp)
{
buffer = 0;
resize(strlen(cp));
strcpy((char*)buffer, cp);
}

ByteArray::ByteArray(unsigned char *cp,unsigned long size)
{
buffer = 0;
resize(size);
memcpy((unsigned char*)buffer, (unsigned char*)cp,size);
}

ByteArray::ByteArray(ByteArray &ba)
{
buffer = 0;
resize(ba.length());
memcpy(buffer, ba.buffer, ba.bufferLength+1);
}

ByteArray::~ByteArray()
{
delete [] buffer;
}

int ByteArray::length() const
{
for (unsigned long i = 0; i < bufferLength; i++)
if (buffer == ''\0'')
return i;

return bufferLength;
}

// implemented in the header
/*bool ByteArray::empty() const
{
return buffer[0] == ''\0'';
}*/

void ByteArray::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 != ''\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;
}
}

void ByteArray::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];
}


void ByteArray::operator+=(const char *data)
{
insert(length(), data);
}

void ByteArray::operator+=(ByteArray & right)
{
insert (length(), (const char *)right.buffer);
}

unsigned char& ByteArray::operator[](unsigned int index)
{
assert (index <= bufferLength); // not required by standard
return buffer[index];
}

void ByteArray::operator=(ByteArray &ba)
{
resize (ba.length());
strcpy ((char*)buffer, (char*)ba.buffer);
}

int ByteArray::operator==(ByteArray &ba)
{
if (bufferLength = ba.bufferLength)
return memcmp(buffer,ba.buffer,bufferLength); //This is wrong...
else
return false;
}

ByteArray ByteArray::operator+(ByteArray &ba)
{
ByteArray clone(*this); // copy left argument
clone += ba; // append right argument
return clone; // return result
}

Share this post


Link to post
Share on other sites
Thanks all. My class works fine AND more importantly I know why. NULL, i''m impressed by that little pointer trick... for a while now I''ve been thinking of memory in terms of bytes rather than types, I guess this really takes the point further with just about everything comming down to pointers and bytes.

All, thanks for your help and advise. In answering the question about why I''m chucking the message''s on the heap. I''ve developed a cool system based on pluggable factories with a message queue in front of it. Things like a keyboard handler generate messages like "h key pressed"(no its not really a string of text). Then the message is placed on a the queue and the pluggable factory work out what to do with it(ie send it to the console or game or menu handler). The handler that recieved the message extract the data, in this example the keycode for the key event and deletes that message.

I''m aware of the possibility of a memory leak if I forget to delete the message and just throw it away. I''m planning on using a template for the handler that automaticall deletes and message it receives after it''s called. All is well.

Again thanks everyone and a special thanks to NULL_POINTER who once again has taught me a whole bunch of stuff about class that I didn''t know (the last time it was inherentance and factories)

gimp
(threads like this should be earmarked and linked to the faq)


Share this post


Link to post
Share on other sites
Double cool.. I just started writing another question about getting my class to allow message[0]=SOMENUMVALUE but ended up working out that if I add an extra constructor for (char ch) and then just cast my enum value I can set it directly...

Let me know if there is a better way thought..

thanks again...

gimp

Share this post


Link to post
Share on other sites

  • Advertisement