0
\$\begingroup\$

Normally I do this:

if (Animal is Dog)
{
    Doc d = (Dog)Animal;
    // d. etc..
} 
else if (Animal is Cat) 
{
    Cat c = (Cat)Animal;
    // c. ....
}

Is this a good way or are there better ways to implement this code above (performance, ...) ?

Should it be like this?:

Dog d = Animal as Dog;

if (d != null;)
{
    // d. etc..
} 
else if (Animal is Cat) 
{
    Cat c = (Cat)Animal;
    // c. ....
}

Or maybe like this?:

Dog d = Animal as Dog;
Cat c;

if (d != null;)
{
    // d. etc..
} 
else if ((c = Animal as Cat) != null) 
{
    // c. ....
}

Or maybe something else?

\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

This question is better suited for Programmers Stack Exchange than Code Review...

To answer your question - if you do evaluations like that it means that most likely you have a design issue at the first place. In most cases you should not branch your logic based on specific type you're working with, since it breaks Open/Closed principle and Liskov substitution principle. Either your class should be a generic class accepting type of the animal to work with, or there must be a virtual method on Animal that should be overridden in Cat and Dog, or something else, depending on specifics of your case.

In most cases the is/as checks are used to check whether a certain object supports an interface or well-known behaviour like:

object message = ....;
if (message is IEnumerable) //now we know that we can iterate on it

or

int Count(this IEnumerable source) //"simplified" version of LINQ's extension method
{
    ICollection collection = source as ICollection;
    if (collection != null)
        return collection.Count;

    int count = 0;
    foreach (var element in source)
        count++;

    return count;
}
\$\endgroup\$
1
  • \$\begingroup\$ asked the question also on Programmers. \$\endgroup\$
    – juFo
    Jun 14, 2013 at 9:07
1
\$\begingroup\$

I agree with almaz' answer that this is likely an architectural problem. But assuming it's not, I would do something like this.

Dog dog = animal as Dog;
if(dog != null)
{
    DoDogStuff(dog);
    return;
}

Cat cat = animal as Cat;
if(cat != null)
{
    DoCatStuff(cat);
    return;
}

The is keyword casts the object to the type you're checking against in order to do the check, then you have to cast it again. The as keyword does the cast only once, then you have to do a null check which is slightly faster. However if you're optimizing for performance this is probably the wrong place to start. These are very small optimizations.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.