Jump to content
  • Advertisement
Sign in to follow this  
gretty

My Custom Container class: wont let me store a member variable pointer?

This topic is 2688 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

Hello

I have a Container class that will allow me to store & retrieve elements from it easily by using the member variables of the elements as their key/identifier.

My problem is I am tring to store a pointer to an objects member variable, but I get a compiler error. What am I doing wrong?
PS: do you have any advice on how I could improve my Container class; such as using const correctly & whether to hide the copy constructor?

This is how I use my container class:

#include <iostream>
#include "ObjectCollection.h"

using namespace std;

// Test items
struct FoodItem
{
unsigned int ID;
string name;
double price;
};

struct Order
{
unsigned int ID;
unsigned int personID;
};


int main()
{
Collection <FoodItem*, std::string> foodCol( &FoodItem::name );
Collection <Order*, unsigned int> orderCol( &Order::personID );

// Create some test objects
string nNames[] = {"a", "b", "c", "d"};
double nPrices[] = {1.1, 2.2, 3.3, 4.4};
unsigned int nPersonIDs[] = {6, 7, 8, 9};

for (int i=0; i<4; i++)
{
FoodItem *f = new FoodItem() { i, nNames, nPrices };
Order *o = new Order() { i, nPersonIDs };

foodCol.store( f );
orderCol.store( o );
}

// Test collection
if ( foodCol["a"]->ID == 0 )
{
cout << "Correctly retrieved a stored FoodItem by its name \n"
}

if ( foodCol[0]->name == "a" )
{
cout << "Correctly retrieved a stored FoodItem by its ID \n"
}

if ( foodCol[7]->ID == 1 )
{
cout << "Correctly retrieved a stored Order by its personID \n"
}

if ( foodCol[1]->personID == 7 )
{
cout << "Correctly retrieved a stored Order by its ID \n"
}


// Release dynamic memory
for (vector <FoodItem*> f=foodCol.getValues(); !f.empty(); f.pop())
{
delete f.back();
}

for (vector <Order*> o=orderCol.getValues(); !o.empty(); o.pop())
{
delete o.back();
}

system("PAUSE");
return 0;
}


My Container implementation: The error occurs in this file:

#ifndef OBJECTCOLLECTION
#define OBJECTCOLLECTION

#include <vector>
#include <map>

template <typename Object, typename dataType>
class Collection
{
public:

Collection( dataType Object::*nMemberVariable )
{
memberVariable = nMemberVariable;
}

~Collection()
{

}

bool store( Object* o )
{
// check that o is not a NULL pointer & not already present in maps
if ( o==NULL || instanceVarMap.find(o->*memberVariable) != instanceVarMap.end() || instanceIntMap.find(o->ID) != instanceIntMap.end() )
{
return false;
}

instanceIntMap.insert( o->ID, o );
instanceVarMap.insert( o->*memberVariable, o );
return true;
}

Object* operator[] ( dataType nParam ) // should this be a const function? I never know when to use const in functions& where not to
{
return get(nParam);
}

Object* operator[] ( unsigned int nParam )
{
return get(nParam);
}

Object* get( dataType nParam )
{
// do I need to check whether the element already exists or does
// stl already do this for me?
if ( instanceVarMap.find(nParam) == instanceVarMap.end() )
{
return NULL;
}

return instanceVarMap[ nParam ];
}

Object* get( unsigned int nParam )
{
if ( instanceIntMap.find(nParam) == instanceIntMap.end() )
{
return NULL;
}

return instanceIntMap[ nParam ];
}

bool remove( Object *o )
{
// check we have o in our collection
if ( instanceIntMap.find(o->ID) == instanceIntMap.end() )
{
return false;
}

instanceIntMap.remove( o->ID );
instanceVarMap.remove( o->*memberVariable );
}

int size()
{
return instanceIntMap.size();
}

bool empty()
{
return instanceIntMap.empty();
}

std::vector <Object*> getValues()
{
std::vector <Object*> values( instanceIntMap.size() );
typename std::map <unsigned int, Object*>::iterator it = instanceIntMap.begin();

while ( it != instanceIntMap.end() )
{
values.add( (*it).second );
it++;
}

return values;
}

Object* front()
{
if ( instanceIntMap.empty() ) { return NULL; }

typename std::map <unsigned int, Object*>::iterator it = instanceIntMap.begin();
return (*it).second;

// can I just say... return (*instanceIntMap.begin()).second; // even if the map is empty?
}

Object* back()
{
if ( instanceIntMap.empty() ) { return NULL; }

typename std::map <unsigned int, Object*>::iterator it = instanceIntMap.end();
it--;
return (*it).second;
}

private:
dataType Object::* memberVariable;
std::map <dataType, Object*> instanceVarMap; // what would be more efficient; a Map or unordered_map? I heard that
std::map <unsigned int, Object*> instanceIntMap; // ...unordered_maps use more memory & Maps are better when integers are the keys
// so should I use maps or unordered_maps?

// I haven't really worked with handling copy constructors
// But do you think its wise I make it private because I am storing pointers?
Collection( const Collection &c )
{

}

Collection( Collection &c )
{

}

Collection& operator =( const Collection &c )
{

}

Collection& operator =( Collection &c )
{

}

};


#endif // OBJECTCOLLECTION





Share this post


Link to post
Share on other sites
Advertisement
The compile error is:

creating pointer to member of non-class type `FoodItem*'
[/quote]

This error occurs when I use this line in main:
Collection <FoodItem*, std::string> foodCol( &FoodItem::name );

And then tells me that the Container constructor & the line...
private:
dataType Object::* memberVariable;

are where the error refers to

Share this post


Link to post
Share on other sites
In general, give as much information as you reasonably can. The full error message including the compilation backtrace, line numbers and so on will make it easier for people to help you.

However, we might have enough information now.

Note your constructor:


Collection( dataType Object::*nMemberVariable )
{
memberVariable = nMemberVariable;
}


Let's instantiate this template "in our heads" as would be done at the start of main(), by replacing 'Object' with 'FoodItem*' and 'dataType' with 'std::string'. We get:


Collection( std::string FoodItem*::*nMemberVariable )
// ^???
{
memberVariable = nMemberVariable;
}


A 'FoodItem*' is obviously a pointer type and so has no member variables as this template instantiation is trying to suggest. So the resulting instantiation is ill-formed.

I suspect the full compiler error message more-or-less says exactly what I've said here, though in a somewhat terser form.

In order to fix this problem, you either need to rework your Collection template so that the first argument can be supplied in non-pointer form, or use something like 'typename boost::remove_pointer<Object>::type' in place of 'Object' in situations such as this.

Share this post


Link to post
Share on other sites
Hi

I have understand most of what your saying except for the most important part :P

In order to fix this problem, you either need to rework your Collection template so that the first argument can be supplied in non-pointer form, [/quote]

How would I be able to do this? See I have made functions where I pass a pointer to member function before (but not stored it) for example:


template <typename T>
void bar( T& obj, int T::*member )
{
std::cout << obj.*member << std::endl;
}


But I am still not clear on how I can accomplish what I want to do.
Something like this doesn't work (I am just fooling around now):

template <typedef Object, typedef dataType>
class Container
{
Container( dataType Object::& memberVar ) {}


// OR

template <typedef Object, typedef dataType>
class Container
{
dataType Object::& memberVariable;

Container( dataType Object::* memberVar ) {}


Share this post


Link to post
Share on other sites

Hi

I have understand most of what your saying except for the most important part :P

In order to fix this problem, you either need to rework your Collection template so that the first argument can be supplied in non-pointer form,


How would I be able to do this?
[/quote]

Try this:


int main()
{
Collection<FoodItem, std::string> foodCol(&FoodItem::name);
// ^^^^^ not a pointer
}


You might be left with a few remaining compiler errors to work through, but now "Object" will be "FoodItem" in your first template instantiation in main(), meaning that "dataType Object::*nMemberVariable" should make sense.

Share this post


Link to post
Share on other sites
Thanks

I really appreciate your help. This is a good learning project for me, but I also intended to use this in my games to easily access a specific item in a container.

Do you know of a way I can acheive what I want to do without having to write a custom Container class for each object? I dont want to sound like I am trying to put a square peg in a round hole :P, but there must be a way to acheive this with persistant objects(pointers to objects) & not just with local objects?

Share this post


Link to post
Share on other sites
Yes you should improve const-correctness. In particular you should look into making many of the methods const by placing a const at the end of the function signature, for the methods that do not change the internal state of the class. (size, empty etc)
Array operators [] should provide const and non-const versions.
Remove the empty destructor, it is superflous to provide an empty non-virtual destructor.
You should remove the non-const copy constructor (well I guess that makes it a move-constructor really), and same goes for them non-const assignment operator. Implementing them does not block anything auto-generated because the compiler does not auto-generate those non-const versions.
Make them private if you intend to make the class non-copyable. Your other choice is to leave them public but toa actually implement a safe way for them to be copied.


Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!