Skip to main content
gamedev.net
🔒 Locked

c++ oploaded operators

Started by Seoushi 14 replies 2k views
Original Post
Seoushi
Seoushi
I know this is just a basic c++ thing but I haven't used it in such a long time and I can't find what I'm looking for on google. Here is the relevent code: cMap.cpp 

//allocate entities
entities = (cEntity*)malloc(sizeof(cEntity) * numEnts);

//load entities
for(unsigned int i = 0; i < numEnts; i++)
{
	getline(in,str);
	entities = new cEntity(str);
}
Basically that code snipet is where its reading the names of the entites from the file and then it loads them. cEntity.h 

class cEntity
	{
		public:
			cEntity(std::string filename); // filename to charset
			~cEntity();

			
			cEntity& operator=(cEntity const&);
	};
thats not the whole class but its all you really need to know. cEntity.cpp 

cEntity& cEntity::operator=(cEntity const& newEntity)
{
	return *this;
}
theres my operator from the entity. Ok so now you have enough information (hopefully) to help with this problem. During compilation (g++ under linux) I get the following error. g++ -c engine/cMap.cpp -I/usr/include/SDL -O2 -pipe -Wall -ansi -o engine/cMap.o engine/cMap.cpp: In member function `void cMap::loadMap(char*)': engine/cMap.cpp:91: error: no match for 'operator=' in '*(((cMap*)this)->cMap::entities + (+(i * 60u))) = (string(((const std::basic_string<char, std::char_traits<char>, std::allocator<char> >&)((const std::basic_string<char, std::char_traits<char>, std::allocator<char> >*)(&str)))), (((cEntity*)operator new(60u)), (<anonymous>->cEntity::cEntity(<anonymous>), <anonymous>)))' engine/cEntity.h:30: note: candidates are: cEntity& cEntity::operator=(const cEntity&) make: *** [engine/cMap.o] Error 1 If you need more code/information then tell me and I will post it. Thanks for the help. -Seoushi
Fruny - Don't put extra-long lines in blocks. Use <tt>...</tt> instead.
Fruny
Fruny
Ugh, I really don't like Apple's GCC error messages. Of course, GCC 3.4+ are crap too, but that's beside the point...

entities is a cEntity*, thus entities is a cEntity. But new cEntity(str) produces a cEntity, which cannot be assigned to entities.

Proposed fix:

entities = (cEntity**)malloc(sizeof(cEntity*) * numEnts);

or

new (entities+i) cEntity(str)
corresponding cleanup will be (entities+i)->~cEntity(), not delete
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan
Seoushi
Seoushi
I believe my malloc line is correct. In the map class it has "cEntity *entities", what I wish to accompilsh with this is to have an array of entities. The way you proposed "entities = (cEntity**)malloc(sizeof(cEntity*) * numEnts);" is for a double array and I don't wish for that. When I incorperate that code I get the follow error.


g++ -c engine/cMap.cpp -I/usr/include/SDL -O2 -pipe -Wall -ansi -o engine/cMap.o
engine/cMap.cpp: In member function void cMap::loadMap(char*)':
engine/cMap.cpp:85: error: cannot convert cEntity**' to `cEntity*' in assignment
engine/cMap.cpp:91: error: no match for 'operator=' in '*(((cMap*)this)->cMap::entities + (+(i * 60u))) = (string(((const std::basic_string<char, std::char_traits<char>, std::allocator<char> >&)((const std::basic_string<char, std::char_traits<char>, std::allocator<char> >*)(&str)))), (((cEntity*)operator new(60u)), (<anonymous>->cEntity::cEntity(<anonymous>), <anonymous>)))'
engine/cEntity.h:9: note: candidates are: cEntity& cEntity::operator=(const cEntity&)
make: *** [engine/cMap.o] Error 1


as for your second proposal I don't quite understand it, and it doesn't fix my error, infact it give my orignal error again.

Quote:

corresponding cleanup will be (entities+i)->~cEntity(), not delete

Yeah I know, something simular to that will be in my ~cMap() function.

Thanks for your help so far, and for the tt tag.
Fruny
Fruny
I didn't say you should do entities = new (entities+i) cEntity(str);, only new (entities+i) cEntity(str);.

It's a placement new.

"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan
DigitalDelusion
DigitalDelusion
this is how you would do it
//WARNING! horrible dealings with raw memory ahead    int numEnts = 10;    cEntity *entities = (cEntity*)malloc(sizeof(cEntity) * numEnts);    for(unsigned int i = 0; i < numEnts; i++)    {     	getline(in,str);	    new (entities + i) cEntity( i);    }        //here's how to finally kill of the entities    for(int i = 0; i != numEnts; ++i)    	entities.~cEntity();    free( entities);


I can't say I see your reason for dealing with memory yourself instead
of using 
cEntities = new cEntitiy[numEnts];
and then doing 
 entities = cEntity( str);
and finally cleaning with 
delete [] entities;
but Im sure you have a good one :) toying around with raw memory can be both fun and yield some nice speedups sometimes.

edit: darn to slow, fruny is teh shit as usual.
HardDrop - hard link shell extension."Tread softly because you tread on my dreams" - Yeats
Seoushi
Seoushi
To fruny:

Never seen that before, I'll have to look it up. It does compile now now I gotta find out about my runtime error :) thanks for the help.

to DigitalDelusion:

I've always just malloc'ed it and I haven't run into problems yet. but yeah I could do it that way as well. I took a class this semester and code speed was a big part of the grade so malloc seems more natural to me right now.
Fruny
Fruny
DigitalDelusion - you can't pass constructor parameters to an array-new

Though... why use malloc and free instead of 
entities = (cEntity*) ::operator new(sizeof(cEntity) * numEnts);::operator delete(entities);


[smile]
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan
Jingo
Jingo
std::vector is your friend. :)
Qw3r7yU10p!
Qw3r7yU10p!
It looks as though you are doing this because it seems more efficient.

I would go for code clarity first and efficieny second. You could easily have written this as follows without having to go and ask for help on the internet to get it to work. Also, even if you get it to work, revisiting it will take a while to understand what it's doing. Don't produce write-only code.Try to write code which is easy to read and comprehend.

std::vector<cEntity> entities(numEnts);//load entitiesfor(unsigned int i = 0; i < numEnts; i++){  getline(in,str);  entities = cEntity(str);}
Seoushi
Seoushi
thanks again guys.. seems I'm having other issues now but I think I have decided that I am going to use std::vector, being clean with my code is really the main reason for this, thanks for reminding me pete. I do have a different question now however, is there an easy way to make a double array using std? if so that could potentialy clean up some really nasty stuff I've been doing.
DigitalDelusion
DigitalDelusion 
std::vector<std::vector<T> > ugh;
works well for jagged arrays for square arrays I would advice on doing a thin wrapper instead.
HardDrop - hard link shell extension."Tread softly because you tread on my dreams" - Yeats
Seoushi
Seoushi
well everything I am using is a rectangluar array so that would work just fine. you would access it like ugh[x][y]; right?
DigitalDelusion
DigitalDelusion
Quote:
Original post by Seoushi
well everything I am using is a rectangluar array so that would work just fine. you would access it like ugh[x][y]; right?


well I would advice doing multi-dimensional array accesses using [y][x] since that's how memory usually is laid out in C... but yes that synxat would work the problem is when resizing the array you need to first resize the outer vector then go through all the children resizing them to like:

struct resize_t{	size_t n;	resize_t(size_t n):n(n){}	template <typename T>	void operator()(T &t){ t.resize( n);}};template <typename T>void resize(std::vector<std::vector<T> > &v, size_t rows, size_t cols){	v.resize( rows);	std::for_each( v.begin(), v.end(), resize_t( cols));}int main(void){	std::vector<std::vector<int> > vv;	resize( vv, 3, 7);}
HardDrop - hard link shell extension."Tread softly because you tread on my dreams" - Yeats
Zahlman
Zahlman
Just as 'delete' cleans up the pointed-at data rather than the pointer itself, 'new' returns a pointer to the new object rather than the object itself (thus your error is about trying to store a pointer to X into an array of Xs). Of course, you can get around that by making use of placement new, but this is filled with a variety of pitfalls, especially as regards how you clean things up. And the net effect anyway is just to allocate enough memory for the whole array and then call the constructor for each element - and as it happens, this is already exactly what is done for you if you simply use an array new - thus speed simply isn't an issue here. (One valid reason to use this technique would be to control just where the memory is being allocated from, or the allocation policy - which *might* help you out with speed actually - but in this case you really want to read this too.

Of course, you should always be careful to delete what you new and delete[] what you new[], exactly once, just like you must free what you malloc. (The difference is very important, because delete[] is "magic" at runtime and plain delete is allowed to do some magic at compile-time precisely because it doesn't need delete[]'s runtime magic). And in the case of placement new into memory allocated by malloc, explicitly call the destructor, don't delete (because you didn't "really" new) and instead free.

As for dealing with multi-dimensional arrays, you have the options cited by the other posters, but you really should read this too.

And in case my multiple links to the same base site haven't driven home the point yet - The C++ FAQ Lite 0wnz j00.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.