[SOLVED] How can I avoid casting in this scenario?

I have a base empty interface IBaseAction and an IAction interface that derives from it and contains a single generic method signature "Perform<T>(PerformArgs<T> args).

The IAction interface is implemented by a bunch of small classes / structs like 'Jump', 'Move', 'Projectile', etc. where T is either a point or a GameObject (Shell) depending on whether they modify an object or not.

I have an Action class that has a IBaseAction member field and tries to find through casting whether the _action field is an IAction<Vector2> or an IAction<Shell> so it can pass in the appropriate parameters. i.e.; if _action implements IAction and you pass in a point to it, the Action class will get the position of the Actor and then pass it on to _action.

I do not want to inherit from Action because I have many different types of Actions, such as DelayedAction or RepeatingAction which inherit from Action.

Anyway I can avoid casting altogether?

Code (CSharp):

1. public class Action : IAction<Vector2>, IAction<Actor>
2. {
3.     private IBaseAction _action;
4.  
5.     public virtual void Perform(PerformArgs<Actor> args)
6.     {
7.         var action = _action as IAction<Actor>;
8.         if (action != null)
9.         {
10.             action.Perform(args);
11.         }
12.         else
13.         {
14.             var target = args.Target.transform.position;
15.             (_action as IAction<Vector2>).Perform(new PerformArgs<Vector2>(args.Actor, target, args.Power));
16.         }
17.     }
18.  
19.     public virtual void Perform(PerformArgs<Vector2> args)
20.     {
21.         var action = _action as IAction<Vector2>;
22.         if (action != null)
23.         {
24.             action.Perform(args);
25.         }
26.         else
27.         {
28.             var target = Targeting.GetActorAtPoint(args.Target);
29.             (_action as IAction<Actor>).Perform(new PerformArgs<Actor>(args.Actor, target, args.Power));
30.         }
31.     }
32. }
33. }

 

Question : What does _action refer to? Isn't the Action class the actual object encapsulating an action? Why does it need to reference another action to execute?

 

Well I have a bunch of Action like DelayedAction, RepeatingAction, etc. and then you can plug anything that impements an IAction interface into them and they will execute.

so you might have a script like 'Move'

public class Move : IAction<Vector2>{ // implement interface}

and then you can put it into the above script or a DelayedAction, RepeatingAction, etc.

 

Mkay so the Action class acts as a proxy for performing the action on an object with an IAction<T> interface? Maybe I'm missing something but I'm not sure you can get away with this without having to cast. I have a similar feature in my framework in which I have to use casting as well although this should not be a performance issue.

 

Yea, basically. Hmm that sucks. I haven't tested it yet but right now the best I've come up with is to cache the cast.

 

Could you add a generic to Action itself and then route instantiation through some kind of factory method? Might get you enough compile-time type enforcement to know what _action is without casting (if you changed _action to IAction<T> maybe; not sure what differs between IAction<T> and IBaseAction or how you're determining that _action is in fact both of them).

Alternately, could you not do the double interface implementation and instead make Perform generic?

It seems a bit weird because you've got a lot of type constraining up until the point where you actually need it.

 

The point of an interface is to abstract away the lower details, so referring to a generic type and just casting to the specific types later is defeating the purpose of the abstraction. See how you haven't removed the dependency on your lower level interfaces?



I don't understand why you want such a generic thing such as IBaseAction. It seems you are trying to create an interface that can represent doing any kind of action to anything. But that's what we have the language for.

Notice that the two lower level IAction's are not really adding anything. Why can't you have Jump simply implement IBaseAction like:

Code (csharp):

1. public class Jump : IBaseAction<Vector2>

This doesn't really solve your problem though, since you will still need to inspect the type of the IBaseAction to see what type to pass to Perform. I think the problem is that you're asking too much of your manager class, which, regardless of your intention, is what the class Action has turned into. You want it to know about and send an arbitrary type to the Perform action without inspecting that type. (Note that what you called casting I would call type checking with a cast.) So I would say no, there is no way to do this without type checking.

However, as you probably already identified, since you're asking about it here, type checking is going to create some ugly code. The reason is what the diagram above shows: you haven't removed Action's dependencies on the lower implementations.

So how can you remove this dependency? I can think of one solution but it introduces a different dependency.. However I think, semantically speaking, you're going to need that dependency anyways.

I can see the temptation to have an IAction like Jump clear of any dependency on a GameObject. After all, it really only wants to work on a point right? However, this is the root of your dependency trouble. By removing the need for Jump to know about GameObject, you require another class to make the decisions about how to get the Jump IAction a point. This is what your Action class is doing now.

This is a bit weird though because we already have a way to operate on points pretty easily. We can just use the + operator directly on Vector2s. So What semantic value would creating a Jump IAction create? There could be a few like: ensuring only y axis forces, encapsulating a jump power or force calculation.

I think this is a little simplistic though, shouldn't a Jump IAction also make sure that whatever is jumping is capable of jumping? And what if my orientation was tilted and the y axis limitation didn't make sense any more. What we would actually like to know is whether the thing that is jumping is on the ground and in which direction the normal is. In order to provide that level of semantic meaning, Jump would need to know about GameObject.

Now, I don't know for sure that all of your implementation of IAction need to know about GameObject but it does reduce the pressure you are placing on your action manager class "Action". Speaking of which, is an odd name. I think ActionManager might be more appropriate.

 

Rather than explicitly checking what kind of argument an action is expecting before you call it, what if you had a base class that did something like:

Code (CSharp):

1. public class Base
2. {
3.     public virtual void Perform(PerformArgs<Actor> args)
4.     {
5.             var target = args.Target.transform.position;
6.             Perform(new PerformArgs<Vector2>(args.Actor, target, args.Power));
7.     }
8.  
9.     public virtual void Perform(PerformArgs<Vector2> args)
10.     {
11.             var target = Targeting.GetActorAtPoint(args.Target);
12.             Perform(new PerformArgs<Actor>(args.Actor, target, args.Power));
13.     }
14. }
15. }

Each version of Perform simply calls the other, so if you actually call Perform on an instance of this class, you'll get an infinite loop. But, a class that derives from this class can override whichever version of Perform is more convenient to work with, and the other version will just call the version that you override.

Then the caller doesn't have to check which version you're expecting.

 

Your logic looks a little weird... looks like you're doing some recursive stuff (without taking a deep look at it). There are some things you can get away with here that will help and will abstract the way you write your code a bit. I believe you could rethink (and internalize) some of your logic and then make the Perform method generic (i.e. Perform<T>(PerformArgs<T> args).

That being said, I wouldn't worry about the casting as your performance impact will be minimal. Since you're dealing with reference types there is no boxing and unboxing occurring and no additional allocation from the cast. However, logically speaking, you are newing up PerformArgs) twice for every call to Perform that takes an Actor. That will allocate every time Perform is called.

I think you're possibly concentrating on the wrong place in terms of optimization. Don't worry about the casting and instead cache your PerformArgs, possibly using an object pool or internal to your class. You could make them a struct, but you'd then be boxing / unboxing and it's possible there would still be heap allocation as the args do have an actor associated, and there are performance implications to inheriting from an interface with a struct (don't do it).

 

Some relevant information on type casting : http://www.codeproject.com/Articles/8052/Type-casting-impact-over-execution-performance-in

Unless you're doing millions of cast per frame, you're good to go.

 

Thanks everyone for your help!

@DoomSamurai , your chart is very helpful. It helped put my mind at ease.

@Dustin Horne , I completely forgot I had made some actions structs for some reason. Thanks for reminding me. I'll have to do a bit of refactoring and make them classes to avoid any (un)boxing costs.

@Antistone , wow! Brilliant idea! This will save me from casting at all! Genius.

 

To add some reason to what @DoomSamurai just said, there are two primary ways to cast:

Code (csharp):

1.  
2. //Prefix (parenthetical) casting
3. var myFoo = (Foo)bar;
4.  
5. //Direct or "as" casting
6. var myFoo = bar as Foo;
7.  

The difference is that the first method (using parenthetical casting) actually does a type check first to see if it can cast to the specified type. If it can't, then it throws an exception. The second method, Direct or "as" casting, skips the type check and just returns "null" if it can't make the cast. The drawback here is that it only works with reference types (i.e. classes).

 

Yeah I know that's why I use 'as' and then checks if it's null. If I casted directly I'd have to check through 'is' and from what I know 'is' + direct cast is slower than 'as'.

 

Or you'd wrap it in a try / catch, or do something like this:

Code (csharp):

1.  
2. var sourceType = typeof(bar);
3. if(sourceType.IsAssignableFrom(typeof(Foo))
4. {
5.    var myFoo = (Foo)bar;
6. }

This also gets slightly more complicated when you have types with generic parameters and you want to handle them in a generic way (such as you don't know what the type parameter is upfront).

 

The purpose of abstractions isn't to improve code speed, it's to improve code maintenance. Of course it is really important to consider the impact our design decisions have on performance, especially in games (admittedly, I'm pretty bad at considering performance first), but I think it is equally important, if not more so, to consider the impact on maintainability.

One of the tenants of maintainability is low coupling. This is why we introduce abstractions, so that we can detach our classes from knowing about too many others. Creating an interface just to recast objects back into their concrete classes is not productive. The goal of the abstraction is to remove the dependency Action has on the lower level classes such as Jump.

Another tenant is readability. @Antistone I think your approach is ingenious, I never would have thought to do something like this. Which is my point. If I had to extend this code, or fix a bug, I would probably spend 15 to 30 minutes just trying to figure out what the heck this class was doing.

 

I didn't take the time to write them for a forum example, but ideally that class would have comments explaining how it is intended to be used, for exactly that reason.