Is this code bad?

Well, the first question is: Why?

What is the point of the templated ALLOCATOR class?

--
Kory

It is a list class. You can store any type of item in it and check the contents later.

What I'm really interested in is this line:

List[ITEM] = reinterpret_cast *> (ItemEntity);


It compiles and runs but will it crash later?

Here's a bit more info on ALLOCATOR:

template
class ALLOCATOR
{
private:
int MaxItems;
int NumberAllocated;// Never decrement, this is the number of items allocated
ITEM** Items;
TAG_LIST* Tag;

Have you thought about creating an abstract base class that you can derive those objects?

That way you can just make an array of that base class and not worry about using the template. You will only need to reinterpret_cast items in the array if you're trying to call a unique function to that class.

Quote:

Original post by Davaris What is your opion of this code?

You have an enumeration that is supposed to keep track of object types. That immediately raises a red flag.

Quote:

Is it unsafe?

You are casting between unrelated types: even if A and B are related, Foo and Foo generally aren't.

Quote:

Can something similar be done using STL?

Between the C++ Standard Library and the Boost libraries, probably.

But you should first try to explain carefully how you plan on using the pointers that you have stored in List.

Quote:

It compiles and runs but will it crash later?

Depends on what you do later. The only thing you can safely do is cast the pointer back to what it was originally. Trying to use the pointers that are in List directly may lead to Bad Things happening.

In general reinterpret_cast should be treated as a last resort. There are usually better ways of accomplishing the same thing. Certainly you shouldn't be using them all over the place!

In this case I think what you are doing is wrong. I don't really see the point of having an array where each index contains a specific type of list. I hardly think there would be any need to iterate through this array. Even if you do, then the code to do it would be no longer if they were separate variables, given that you have to cast each one to the correct type anyway. It seems to me like they should just be separate lists.

No doubt, what you want to achieve with those 'lists' can be done much better using the C++ standard library.

So how would you guys do it in STL?

I have many lists for each object type and they all inherit from the ENTITY class.

ALLOCATOR * Base;
ALLOCATOR * ItemEntity;
ALLOCATOR * Soldier;
ALLOCATOR * Projectile;
ALLOCATOR * Cycle;
ALLOCATOR * Binary;
ALLOCATOR * Door;
ALLOCATOR * ContainerMap;
ALLOCATOR * Trigger;
ALLOCATOR * Vehicle;

What I want to do is use functions like:

ENTITY* Spawn (const int e_type = ENTITY::BASE)
{
ENTITY* e;

switch (type)
{
case ENTITY::BASE: e = Base->GetNewItem(); break;
case ENTITY::SOLDIER: e = Soldier->GetNewItem(); break;
// etc...
}
}

// and

ENTITY* Select (const int e_type, const int scrn_x, const int scrn_y)
{
// Search through List[e_type] for a match...
}

If I have to search through the lists separately (Base, ItemEntity, Soldier...) I wouldn't be able to write a general purpose search routine for all of the lists.

Virtual functions and a factory. Without knowing more about your design:

Note that ALL_CAPS_WITH_UNDERSCORES is reserved by most coding standards for preprocessor definitions. Using it for class names is probably A Bad Idea™.

EDIT: I originally omitted the Entity destructor because the code was intended as an outline, not a solution. On reflection I decided it was better to include it.

Σnigma

[Edited by - Enigma on June 15, 2006 10:46:23 AM]

>Note that ALL_CAPS_WITH_UNDERSCORES is reserved by most coding standards for preprocessor definitions. Using it for class names is probably A Bad Idea™.

Yes this is a very old engine. I'm trying to bring it and myself out of the dark ages. :)

So I have to get Boost as well? Man I've got a lot to learn. :)

You put everything in the same list. So when I destroy something I'd have to delete it. Would there be any problems with memory fragmentation? Hmmm... Perhaps I could add another variable to the entity class indicating whether or not it is in use.