5

I have a class of 3 different linked lists (for saving the entities in a game I'm working on). The lists are all of objects with the same base type, but I keep them separate for processing reasons. Note that IEntity, IObject and IUndead all inherited from IEntity.

public class EntityBucket
{
    public LinkedList<IEntity> undeadEntities;
    public LinkedList<IEntity> objects;
    public LinkedList<IEntity> livingEntities;

    public EntityBucket()
    {
        undeadEntities = new LinkedList<IEntity>();
        objects = new LinkedList<IEntity>();
        livingEntities = new LinkedList<IEntity>();
    }

    public LinkedList<IEntity> GetList(IObject e)
    {
        return objects;
    }

    public LinkedList<IEntity> GetList(IUndead e)
    {
        return undeadEntities;
    }

    public LinkedList<IEntity> GetList(ILiving e)
    {
        return livingEntities;
    }

}

I have 3 methods for retrieving each of the lists, currently based on their parameters. The fact that there are 3 is fine, since I know each list will in some way or another require its own accessor. Passing an instantiated object is not ideal though, as I may want to retrieve a list somewhere without having an object of similar type at hand. Note that the object here is not even used in the GetList methods, they are only there to determine which version to use. Here is an example where I have an instantiated object at hand:

public void Delete(IUndead e, World world)
{

     .....
     LinkedList<IEntity> list = buckets[k].GetList(e);
     .....
}

I don't like this current implementation as I may not always have an instantiated object at hand (when rendering the entities for example). I was thinking of doing it generically but I'm not sure if this is possible with what I want to do. With this I also need 3 Delete methods (and 3 of any other, such as add and so forth) - one for each type, IUndead, IObject and ILiving. I just feel that this is not the right way of doing it.

I'll post what I have tried to do so far on request, but my generics is rather bad and I feel that it would be a waste for anyone to read this as well.

Finally, performance is very important. I'm not prematurely optimizing, I am post-optimizing as I have working code already, but need it to go faster. The getlist methods will be called very often and I want to avoid any explicit type checking.

2
  • What are the parameters for in the GetList methods if they're not actually used? It seems like you could just expose the lists as properties rather than methods. Commented May 16, 2012 at 20:13
  • That's how I used to do it, but as I stated, I have to have 3 versions of each method that access these lists. I don't want 3, I'd rather want one generic version. This cannot be done by explicitly calling the list as a property. The lists themselves need to be retrieved in a generic way. The correct list to choose will have to be determined using some Type or another. Commented May 16, 2012 at 20:25

4 Answers 4

3

So you want a better interface, because, as you said, passing an unnecessary object to GetList just to figure out its type makes little sense.

You could do something like:

public List<IEntity> GetList<T>() : where T:IEntity
{
    if(typeof(T)==typeof(IUndead)) return undedEntities;
    // and so on
}

And you'll have to call it like this: GetList<IUndead>();

I think an enum is a better idea here:

enum EntityTypes { Undead, Alive, Object };
public List<IEntity> GetList(EntityTypes entityType) { ... }

It's cleaner and makes more sense to me.

EDIT: Using generics is actually not that simple. Someone could call GetList a Zombie type, which implements IUndead, and then you'll have to check for interface implementations. Someone could even pass you a LiveZombie which implements both IUndead and IAlive. Definitely go with an enum.

Sign up to request clarification or add additional context in comments.

6 Comments

I agree with making an enum for this situation.
I like your answer. I will implement it before accepting it though. I guess I haven't mentioned this in my post, but performance really is an issue here. I'm slightly afraid to do typeof, as these GetList functions will be called thousands of times per second, and according to msdn.microsoft.com/en-us/library/ms973858.aspx, "Type comparison operations—in this case, typeof in C#, GetType, is, IsInstanceOfType and so on—are the cheapest of the reflection APIs, though they are by no means cheap." Note the "by no means cheap"
If performance is an issue, enums will beat anything that uses reflection.
Cool thanks. This seems the way to go then. Time to scratch up on my enums :). Grabbing some tutorials now. I'll be back in an hour or so and then I'll accept your answer if someone didn't come up with a better solution (not that their is anything wrong with yours! Just curious to see what clever tricks are out there). Thanks a lot though
This is actually pretty good. Need to think of how exactly I'll incorporate this into the bigger picture, but that's my job, right? :P thanks again
|
1

How about a better implementation to go with that better interface?

public class EntityBucket
{
  public LinkedList<IEntity> Entities;

  public IEnumerable<T> GetEntities<T>() where T : IEntity
  {
    return Entities.OfType<T>();
  }

}


List<IUndead> myBrainFinders = bucket.GetEntities<IUndead>().ToList();

With this implementation, the caller better add each item to the right list(s). That was a requirement for your original implementation, so I figure it's no problem.

public class EntityBucket
{
  Dictionary<Type, List<IEntity>> entities = new Dictionary<Type, List<IEntity>>();

  public void Add<T>(T item) where T : IEntity
  {
    Type tType = typeof(T);
    if (!entities.ContainsKey(tType))
    {
      entities.Add(tType, new List<IEntity>());
    }
    entities[tType].Add(item);
  }

  public List<T> GetList<T>() where T : IEntity
  {
    Type tType = typeof(T);
    if (!entities.ContainsKey(tType))
    {
      return new List<T>();
    }
    return entities[tType].Cast<T>().ToList();
  }

  public List<IEntity> GetAll()
  {
    return entities.SelectMany(kvp => kvp.Value)
      .Distinct() //to remove items added multiple times, or to multiple lists
      .ToList();
  }

}

2 Comments

Thanks, I will implement this as soon as I have time. If it's fast enough I will return here
But after having a look at this it looks more or less like what I have had in mind.
1

How about something like the following?

public LinkedList<IEntity> GetList(Type type) {
    if (typeof(IUndead).IsAssignableFrom(type)) return undeadEntities;
    if (typeof(ILiving).IsAssignableFrom(type)) return livingEntities;
    if (typeof(IObject).IsAssignableFrom(type)) return objects;
}

Then you would call it like this:

var myUndeads = GetList(typeof(IUndead));
var myLivings = GetList(typeof(ILiving));
// etc

The same type of logic could be implemented in your deletes, add, and other methods, and you never need a concrete instance of an object to access them.

The IsAssignableFrom logic handles subclassing just fine (i.e. you could have a CatZombie, which derives from Zombie, which implements IUndead, and this would still work). This means you still only have to create one Delete method, something like the following:

public void Delete(IEntity e, World world) {
    if (typeof(IUndead).IsAssignableFrom(type)) undeadEntities.Remove(e);
    if (typeof(ILiving).IsAssignableFrom(type)) livingEntities.Remove(e);
    if (typeof(IObject).IsAssignableFrom(type)) objects.Remove(e);
}

EDIT: I noticed your comment on zmbq's answer regarding performance; this is definitely NOT fast. If you need high performance, use an enum-style approach. Your code will be more verbose and require more maintenance, but you'll get much better performance.

6 Comments

Yeah sorry about that. I'll give you one up for the thought though. I better update my post to reflect this! Sorry once again
This is actually worse than using generics. If you use generics, the compiler will catch you if you pass a String or a DataSet. And you're still facing the trouble of getting an object that implements IAlive AND IUndead.
@zmbq "And you're still facing the trouble of getting an object that implements IAlive AND IUndead." I'm not sure what you mean; Delete will handle the multiple interfaces just fine. Are you talking about GetList not handling multiple interfaces? In that case your solution doesn't solve the problem either. The fix for that isn't hard though, you just need to combine the results from all relevant interfaces (your solution would need to do that as well)
I'm talking about passing an object that implements both IAlive and IUndead. Do you remove it from undeadEntities? livingEntities? Both? None of them? With the enum, that's impossible.
@zmbq I'm confused. Why are you arguing against using an enum? I didn't propose that idea, you did. My solution doesn't use enums, nor does it have any of the problems you're talking about. However, my solution is slow, which is where enums win.
|
0

Seems to me you could just implement a Dictionary of named LinkedList's and refer to them by name or enum.

That way adding or removing lists is just an implementation issue and no separate class to deal with.

1 Comment

That's how I had it initially, but my code expanded to have these 3 different types, that I need to be able to handle separately. That's why I added the bucket with 3 linked lists. I have also tried making 3 dictionaries, but that was the slowest implementation by far, as dictionary accesses are relatively slow.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.