#42877 - sajiimori - Sun May 15, 2005 10:37 pm
I have a class hierarchy. I want to prevent most people from creating objects in the hierarchy, but grant that right to one object. The creator object should be able to create anything in the hierarchy, but should know about the base and only the base.
The following doesn't work because Derived can't access the Base constructor. Obviously, I wouldn't want Derived to be a friend of Base.
| Code: |
class Creator;
class Base
{
private:
Base();
friend class Creator;
};
class Derived : public Base
{
};
class Creator
{
public:
// This could be overloaded to take constructor arguments.
template<class T>
T* create()
{
T* t = new T;
method(t);
return t;
}
private:
void method(Base*)
{
}
};
void test()
{
Derived* d = Creator().create<Derived>();
}
|
#42883 - MumblyJoe - Mon May 16, 2005 2:21 am
Alright, as a first stab in the dark, I have had a play around. Remember that friendship does not get inherited - just because you have a friend does not mean that your kids will be friends with them.
Here's my concept, it mildly changes the interface, in particular bieng that the constructor in Base in protected instead of private, and now the whole Creator class is a template instead of just the create function. Also, all classes that derive from Base must say they are friends of Creator<whatever>, and I'm not sure what happens when you derive from Derived. Seems to work with gcc and msvc.
| Code: |
template<class T>
class Creator;
class Base
{
protected:
Base(){};
};
class Derived : public Base
{
private:
Derived(){};
friend class Creator<Derived>;
};
template<class T>
class Creator
{
public:
T* create()
{
T* t = new T;
method(t);
return t;
}
private:
void method(Base*)
{
}
};
void test()
{
Derived* d = Creator<Derived>().create();
}
int main()
{
test();
return 0;
}
|
_________________
www.hungrydeveloper.com
Version 2.0 now up - guaranteed at least 100% more pleasing!
#42888 - sajiimori - Mon May 16, 2005 5:06 am
Hmm... the game would really essplode if somebody accidentally circumvented the system, e.g. by holding one of the objects by value. Since there are many classes in the hierarchy, and many of them create other objects, "protected" isn't very protected at all.
I hope there's some way to do this. I already got a "random" crash from it today.
#42889 - MumblyJoe - Mon May 16, 2005 7:01 am
This will look really wierd until you have looked at it twice, but I think it's closer to the solution, however still not the most elegant or maintainable code possible, and it makes the idea of deriving again from Derived leave a bad taste in my mouth:
| Code: |
template<class B, class T>
class Creator
{
public:
static T* create()
{
T* t = new T;
method(t);
return t;
}
private:
static void method(B*){}
};
class Base
{
public:
Base(){};
private:
virtual void base_ctor_protection()=0;
};
class Derived : public Base, public Creator<Base,Derived>
{
private:
Derived(){};
virtual void base_ctor_protection(){};
friend class Creator<Base,Derived>;
};
void test()
{
Derived* d = Derived::create();
}
int main()
{
test();
return 0;
}
|
_________________
www.hungrydeveloper.com
Version 2.0 now up - guaranteed at least 100% more pleasing!
#42902 - jma - Mon May 16, 2005 3:18 pm
| sajiimori wrote: |
| I have a class hierarchy. I want to prevent most people from creating objects in the hierarchy, but grant that right to one object. The creator object should be able to create anything in the hierarchy, but should know about the base and only the base. |
After the initial gut-wrench from looking at the code, my first impulse is that you are going about this completely the wrong way (or perhaps I just don't understand your problem). Can you put this into a context?
However, I'd suggest making all available to the end-programmer, but just make Base worthless by itself. For example:
| Code: |
// -- interface.h
class Base {
protected:
// data members inaccessible to outside
};
class Interface {
// no data members, and worthless
public:
// all virtual methods here
};
// external access class
class Derived : public Base, public Interface {
// has data members of Base
// has access methods of Interface
}; |
Now just use new and delete normally. Someone can make instances of Base and Interface, but they are worthless alone. Derived (or a derivative thereof) is needed to do anything useful.
Another quick question: why does Creator have to be a class? If you want to create the objects in some unique way (from a pool for example), just overload new. Or, you can always just template a global function, too:
| Code: |
template <class T> T* Create() {
// blah blah code
return new T;
}
int main() {
int *x = Create<int>();
} |
Hope this helps,
Jeff M.
_________________
massung@gmail.com
http://www.retrobyte.org
#42905 - poslundc - Mon May 16, 2005 4:00 pm
Yeah, I am having trouble understanding the nature of the problem as well. Perhaps you could provide some context as to why you would want to set up a system like this?
At first glance, it looks like the only point of forcing other programmers to use the Creator class to instantiate the objects is to make sure method() is being called on the base class after construction. But if this kind of post-construction behaviour were really necessary, you could just provide a separate Init() function in the base class and assert somewhere if Init() never gets called. That, to me, would be a lot less error-prone than attempting to automate the process with code that is difficult to understand/maintain.
Dan.
#42919 - sajiimori - Mon May 16, 2005 7:00 pm
This is almost one of those "you'd have to be there" sort of problems.
All the objects in the hierarchy are conceptually self-managing: they do their work for as long as they exist, and then they die on their own. (If you think this is a bad way to do things, then stop reading now because I don't care to hear arguments against it.)
To let the objects live independently of the client, they must be added to a list of all such objects, which will automatically be traversed every tic.
| Code: |
| gList.add(new Derived); |
This is repetitive and error prone, especially if you didn't know the object is supposed to go in the list, and especially if the object expects to be in the list.
The solution is to have the base class automatically add itself to the list.
| Code: |
| Base::Base() { gList.add(this); } |
That makes the client code look like this:
First off, that's a little weird. It looks like a memory leak. Second, if somebody mistakenly thought that the object was derived from Base (or if it was but later wasn't), then they really did create a memory leak.
The solution is to have a creation mechanism that must be used for objects in the hierarchy, but cannot be used for other objects. Then the compiler will catch the errors I've mentioned.
Dan: As always, I'm apt to initially invest in a system to simplify client code. My level of investment is a function of how widely the system will be used and how much better each usage case will be on average.
#42921 - poslundc - Mon May 16, 2005 8:07 pm
I would just define a static Create() method in each of the subclasses that calls the protected constructor of the base class, and make their individual constructors private.
This requires more discipline on the part of the person creating the subclasses, to make sure that each derivative has a Create() method. But the objective is to reduce the workload on the users of the class, right?
| Code: |
class Base
{
protected:
Base()
{
gList.add(this);
}
};
class Derived : public Base
{
public:
static void Create()
{
new Derived;
}
private:
Derived()
{
}
};
int main(void)
{
Derived::Create();
} |
(Coded from the hip)
Dan.
#42935 - sajiimori - Tue May 17, 2005 12:27 am
As I mentioned earlier, due to the large number of classes in the hierarchy and their tendancy to create other derivatives, "protected" does not provide encapsulation. The crash that I already saw was due to one derivative holding another by value.
Also, derived classes are clients of the base, and so the system is for their benefit as much as everyone else's. Like stack frames in assembly or vtables in C, these Create methods may be a thing that the language is incapable of abstracting.
I'm currently using the "new Derived;" method, as weird as it is.
#42937 - poslundc - Tue May 17, 2005 1:05 am
| sajiimori wrote: |
| As I mentioned earlier, due to the large number of classes in the hierarchy and their tendancy to create other derivatives, "protected" does not provide encapsulation. The crash that I already saw was due to one derivative holding another by value. |
In my example, though, constructors of the derived classes are made private. Only the constructor of the base class is protected. Wouldn't this alleviate that problem?
| Quote: |
| I'm currently using the "new Derived;" method, as weird as it is. |
<shrug> It's not so bad. If you're really worried about people misusing the class and trying to delete pointers returned by new, then you might want to make it convention to use "smart" pointers with your class... that way the refcounting would be out of the user's hands.
Dan.
#42939 - sajiimori - Tue May 17, 2005 1:44 am
Hmm, yeah, your convention does keep each class encapsulated individually.
Bleh... the issues are so subtle. Even holding these objects by value or deleting them yourself doesn't normally cause a problem. It only breaks if the list of objects is currently being traversed, and the current object being updated is right next to the one being deleted (which is often the case if one object holds another by value, since they are constructed in sequence). Then it invalidates the linked list iterator. I don't even know what other issues there are.
All I have to say is: Garbage collection! :P
#42940 - poslundc - Tue May 17, 2005 2:46 am
| sajiimori wrote: |
| Bleh... the issues are so subtle. Even holding these objects by value or deleting them yourself doesn't normally cause a problem. It only breaks if the list of objects is currently being traversed, and the current object being updated is right next to the one being deleted (which is often the case if one object holds another by value, since they are constructed in sequence). Then it invalidates the linked list iterator. I don't even know what other issues there are. |
I'd consider usage like that to be programmer error more than a design flaw. (Maybe a flaw in the language, but that's another matter.) You can't protect coders from everything, and you can't protect them from themselves... there are certain logistics to managing things like a list class that you need to learn over time and from experience, I think.
Remember, STL doesn't expect every method to work in every situation. The documentation lists things like what can cause an iterator to become invalidated, but most programmers still need to figure it out for themselves sooner or later.
Dan.
#42942 - sajiimori - Tue May 17, 2005 4:04 am
I started trying to explain the problem fully, but it's too hard.
Briefly: The list is automatically traversed every game loop to update all the objects. The object that is currently being updated can signal that it wants to be deleted. (It doesn't do "delete this" because that would hose the iteration.) After the object is updated, the manager sees that the "delete me" flag is set and actually deletes it. At this point, the iterator is just after the object that wanted to be deleted.
The problem is when the destructor for the object destroys the object that the iterator is now pointing at. This happens implicitly if the object is being held by value, so it's hard to track down.
I agree that it's impossible to protect against anything that could crash the system (in C++), but conventional wisdom says you should hold things by value when possible, so I'd be more inclined to blame myself if somebody followed that guideline contrary to my intended usage.
#42945 - DekuTree64 - Tue May 17, 2005 5:33 am
So, the problem here is to warn the innocent coder who tries to delete an object by value when he should let it die on its own, right? Is there any reason you can't have a private flag in the Base class that only your creator touches, and assert on that in Base's destructor?
Something like:
| Code: |
class Creator;
class Base
{
private:
Base() : wasCreatedProperly(false) {}
~Base() { ASSERT_TEXT(wasCreatedProperly, "Use the creator!"); }
friend class Creator;
bool wasCreatedProperly;
};
class Creator
{
public:
// This could be overloaded to take constructor arguments.
template<class T>
T* create()
{
T* t = new T;
method(t);
return t;
}
private:
void method(Base *thing)
{
thing->wasCreatedProperly = true;
}
}; |
And possibly another to make sure it isn't deleted by anything but your system.
_________________
___________
The best optimization is to do nothing at all.
Therefore a fully optimized program doesn't exist.
-Deku
#42963 - jma - Tue May 17, 2005 3:26 pm
| sajiimori wrote: |
| Briefly: The list is automatically traversed every game loop to update all the objects. The object that is currently being updated can signal that it wants to be deleted. (It doesn't do "delete this" because that would hose the iteration.) After the object is updated, the manager sees that the "delete me" flag is set and actually deletes it. At this point, the iterator is just after the object that wanted to be deleted. |
Are you using reference counting (or something similar) or are you just assuming that a single "delete" should clean the object? Could make a big difference in approaches taken. I'd seriously consider looking at Apple's Cocoa libraries in Objective-C. They did a pretty fine job of this. "Autorelease" being a beautiful method.
Jeff M.
_________________
massung@gmail.com
http://www.retrobyte.org
#42970 - sajiimori - Tue May 17, 2005 5:40 pm
Deku: Hmm, it's a runtime error, but that's a lot better than random crashes or memory leaks. I think I'll use it -- thanks! :)
Ooh, since the creator is the one that iterates through the list, it can even check the flag while iterating to catch it immediately.
Jeff: There's no reference counting.