7

When working with MVVM and Prism i find myself doing a lot of casting, as most parameters are interfaces

Ex

  public void AddCrSubSystemsToPlant(IPlantItem plantItm, CRArticleItem crItm)
        {

            OSiteSubSystem itm = (OSiteSubSystem)crItm;
            itm.PartData.Order = ((OSiteEquipment)plantItm).SubSystems.Count() + 1;

            ((OSiteEquipment)plantItm).SubSystems.Add(itm);

        }

or

  public void DeletePart(IPlantItem plantItem)
        {
            IEnumerable<IPlantItem> itmParent = GetParentPartByObjectId(_siteDocument, plantItem);

            if (plantItem is OSiteEquipment)
            ((ObservableCollection<OSiteEquipment>)itmParent).Remove((OSiteEquipment)plantItem);

            if (plantItem is OSiteSubSystem)
                ((ObservableCollection<OSiteSubSystem>)itmParent).Remove((OSiteSubSystem)plantItem);

            if (plantItem is OSiteComponent)
                ((ObservableCollection<OSiteComponent>)itmParent).Remove((OSiteComponent)plantItem);
        }

My question is , whats the cost involved. Are these operations costly memory or cpu wise, should they be avoided.

Any views?

klashagelqvist
  • 1,251
  • 2
  • 26
  • 52
  • 10
    Why do you need all these casts? Don't your interfaces expose the required operations? If not, why not? – Oded Mar 07 '12 at 15:05
  • 1
    You could probably mock up some test cases with and without casting and measure the performance. I don't think any individual cast is much of a performance hit, but it depends how frequently you do it. – Matt Burland Mar 07 '12 at 15:05
  • See this related answer: http://stackoverflow.com/a/9366456/414076 – Anthony Pegram Mar 07 '12 at 15:06
  • 1
    Consider using the `as` keyword as the casting will not throw an exception, but will return `null` instead. – John Alexiou Mar 07 '12 at 15:07
  • Also you could remove you duplicated casts in your second code snippet by using "as" instead of "is" and then checking for null. Of course, since you are checking multiple types, I'm not sure if that would actually give better performance. [Edit: ja72 beat me to it!] – Matt Burland Mar 07 '12 at 15:07
  • Might be a naive thought but I'm sure WPF itself depends on a lot of casting, but as Matt stated, profiling it will be the best thing to do. – Paulie Waulie Mar 07 '12 at 15:08
  • @svick - I forgot to mention to *always* check for `null` and proceed accordingly without having to interrupt program flow unless it is necessary to do so. I do not advocate hiding it, just avoiding too many unnecessary code terminations. – John Alexiou Mar 07 '12 at 15:40

3 Answers3

7

I think the more important question is why are you doing so much casting?

In the first example:
Why is the first parameter type IPlantItem if you keep casting it to OSiteEquipment? The same can be said about the second parameter.

In the second example:
Why does GetParentPArtByObjectId return an IEnumerable<IPlantItem>? If it were to return an ICollection<IPlantItem> you wouldn't have to cast to ObservableCollection<T>. ObservableCollection<T> inherits from Collection<T> which implements both ICollection<T> and ICollection. You should be able to remove the item from the collection without even knowing its type.

Now some advice.
Don't cast the same object multiple times.
Don't do this:

if (obj is IPlantItem)
    ((IPlantItem)obj).DoSomething();

Do this instead

IPlantItem plant = obj as IPlantItem;
if (plant != null)
    plant.DoSomething();

Use base types whenever possible. This will keep you from needing to cast so much. As previously stated, don't cast to ObserableCollection<T> to call a method on ICollection

Use generics. If you need type specific logic, make an abstract base class(or just an interface if you don't need any shared logic) with a generic parameter. Then make implementations of that class for each of the implementations of the interface. Methods can be generic, too. I can rewrite the second example as

public void DeletePart<TPlantItem>(TPlantItem plantItem)
    where TPlantItem : IPlantItem
{
    IEnumerable<TPlantItem> itmParent = GetParentPartByObjectId(_siteDocument, plantItem);
    ((ObservableCollection<TPlantItem>)itmParent).Remove(plantItem);
}
cadrell0
  • 17,109
  • 5
  • 51
  • 69
  • Good luck with unit testing when the implementation requires an actual implementation and not the behaviour of an interface. – weismat Mar 07 '12 at 15:36
  • Which is why I recommended changing parameter types instead of casting and generic abstract base classes / interfaces with type specific implementations. – cadrell0 Mar 07 '12 at 15:39
  • Even though you gave the OP what he needed, I think giving him what he wanted (“What's the cost of casting?”) at least in short would be good. – svick Mar 07 '12 at 15:42
  • @svick Anthony Pegram took care of that by linking to this answer http://stackoverflow.com/a/9366456/414076 – cadrell0 Mar 07 '12 at 15:45
  • Thanks for your answer. I would like to implement ICollection but sadly it seems i cant cast from ObservableCollection to ICollection even if OSiteEquipment implements interface IPlantItem – klashagelqvist Mar 07 '12 at 15:51
  • Try casting down to `ICollection` instead of `ICollection`. It probably has to do with covariance, which is a whole other topic when dealing with generics. – cadrell0 Mar 07 '12 at 15:56
1

use

       ((System.Collections.IList)itmParent).Remove(plantItem);

instead of

        if (plantItem is OSiteEquipment) 
        ((ObservableCollection<OSiteEquipment>)itmParent).Remove((OSiteEquipment)plantItem); 

        if (plantItem is OSiteSubSystem) 
            ((ObservableCollection<OSiteSubSystem>)itmParent).Remove((OSiteSubSystem)plantItem); 

        if (plantItem is OSiteComponent) 
            ((ObservableCollection<OSiteComponent>)itmParent).Remove((OSiteComponent)plantItem); 
Serj-Tm
  • 16,581
  • 4
  • 54
  • 61
0

This article might shed some light on how casting affects performance

http://www.codeproject.com/Articles/8052/Type-casting-impact-over-execution-performance-in

Here are some general tips for optimizing your programs based on the results obtained in the previous sections:

  • Numeric type conversions are usually expensive, take them out of the loops and recursive functions and use the same numeric types when possible.

  • Downcasting is a great invention but the type checks involved have a great impact on execution performance, check the object types out of loops and recursive functions, and use the "as" operator into them.

  • Upcasting is cheap!, use it everywhere you need.

  • Build lightweight conversion operators to make custom casts faster. Tools used

Community
  • 1
  • 1
Ani
  • 10,826
  • 3
  • 27
  • 46
  • I have found that most of these principles and results hold although the numbers may be outdated. – Ani Mar 07 '12 at 15:09