Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Quite recently, I thought about the use of as, is and direct cast in C#.

Logically, is it a better idea to use :

 var castedValue = value as type;
 if (null != castedValue)
 {
     // Use castedValue.
 }

than :

if (value is type)
{
    var castedValue = (type)value;
    // Use castedValue.
}

But I still have issues when dealing with this kind of pattern:

if (value is Implementation1)
{
    var castedValue = (Implementation1)value;
    // Use castedValue in a first way.
}
else if (value is Implementation2)
{
    var castedValue = (Implementation2)value;
    // Use castedValue in a second way.
}
else if (value is Implementation3)
{
    var castedValue = (Implementation3)value;
    // Use castedValue in a thrid way.
}
// And so on...

How can I improve this code to prevent casting two times when OK, and is it really necessary to cast twice?

I don't want to make the code unreadable, but the idea is not to test a cast if a previous one succeeded.

I had several ideas to fix this, but none of them seems to really satisfy the conditions...

Edit:

Here is a case I've got... There is this object that is created by a lower part of teh code I do not have control on. This object can be of several inherited types. In my layer, I do want to create a specific object depending on this type. I do have this constructor class:

public static IHighLevelObject MakeHighLevelObject(LowLevelObject lowLevelObject)
{
    IHighLevelObject highLevelObject;

    if (lowLevelObject is LowLevelObject1)
    {
        highLevelObject = new HighLevelObject1((LowLevelObject1)lowLevelObject);
    }
    else if (lowLevelObject is LowLevelObject2)
    {
        highLevelObject = new HighLevelObject2((LowLevelObject2)lowLevelObject);
    }
    else if (lowLevelObject is LowLevelObject3)
    {
        highLevelObject = new HighLevelObject3((LowLevelObject3)lowLevelObject);
    }
    // And so on...

    return highLevelObject;
}

How do I solve this case ?

share|improve this question

migrated from codereview.stackexchange.com Feb 28 '13 at 11:59

This question came from our site for peer programmer code reviews.

3  
What is your problem with the code above? If it's stylistic, I'm afraid there isn't much you can do about it. If you're worried about the possible correctness/performance implications of casting multiple times, remember that the is operator doesn't perform a cast, but is rather a low-level MSIL instruction that is optimized and efficient. –  Avner Shahar-Kashtan Feb 28 '13 at 12:37
    
My question was indeed a performance issue question. If is is optimized and efficient, maybe my first assumption about using as instead of is + cast is not so true, or does not make a real difference... –  clemchen Feb 28 '13 at 14:33
    
Check out the accepted answer here for a related discussion. stackoverflow.com/a/2139818/701054 –  Avner Shahar-Kashtan Feb 28 '13 at 14:43
    
That is precisely the kind of discuss that made me think about the given problem: as would get better performance here, but is is much more convinient to use... –  clemchen Feb 28 '13 at 14:56
2  
What will happen if a new LowLevelObjectN class will be created that previously didn't exist? –  almaz Feb 28 '13 at 21:28

2 Answers 2

up vote 15 down vote accepted

This kind of code suggests design issues that you need to tackle first.

The code breaks Open-Closed Principle and Liskov substitution principle: whenever you need to add a new implementation type to extend the functionality you'll have to edit this class. You should design your solution so that logic branching is done via generalized approach, e.g. using polymorphism. Since you have provided a very general description it is hard to suggest appropriate solution for your problem.

share|improve this answer
7  
True, and might solve the problem in a given case, but not always and there is no attempt to actually answer the question here. –  pdr Feb 28 '13 at 12:15
2  
@pdr Question: How can I improve this code to prevent casting two times. My answer suggests approach that will prevent both casts. –  almaz Feb 28 '13 at 13:16
1  
@almaz: That's not the question at all. The question is: what is the best approach to safe and direct casting, where the number of conditions is greater than one. I can think of a case I have where that question could be asked and your answer offers nothing. Cause I already thought of that. I think Avner's comment should be an answer and your answer a comment. –  pdr Feb 28 '13 at 13:25
1  
@pdr I've added the answer while question was still in CodeReview. I reviewed the code and highlighted the root issue. –  almaz Feb 28 '13 at 13:48
1  
@clemchen - Yes, you need to. You just need an abstract/virtual function on the base class, and the correct implementation on each subclass. –  Bobson Feb 28 '13 at 16:44

Assuming polymorphism is nowhere on the radar, you can use multiple returns and use as.

public static IHighLevelObject MakeHighLevelObject(LowLevelObject lowLevelObject)
{
    LowLevelObject1 object1 = lowLevelObject as LowLevelObject1;
    if (object1 != null)
    {
        return new HighLevelObject1(object1);
    }
    LowLevelObject2 object2 = lowLevelObject as LowLevelObject2;
    if (object2 != null)
    {
        return new HighLevelObject2(object2);
    }
    // And so on...
}

Your LowLevelObject class could just have a virtual ToHighLevelObject method that each subclass overwrites and avoid this nastiness to begin with.

share|improve this answer
    
+1, thanks, I'll mark almaz as answer for the general case. For the particular case, this indeed works, I didn't see this simple solution for my wanting to have one return place, but here this seems better. –  clemchen Mar 1 '13 at 8:07

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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