Best "not is" syntax in C# [closed]

Closed. This question is off-topic. It is not currently accepting answers.

---

Want to improve this question? Update the question so it's on-topic for Stack Overflow.

Closed 11 years ago.

Improve this question

It is important to me that my syntax do开发者_运维技巧es not make other developers confused.

In this example, I need to know if a parameter is a certain type.

I have hit this before; what's the most elegant, clear approach to test "not is"?

Method 1:

void MyBinding_Executed(object sender, ExecutedRoutedEventArgs e)
{
    if (!(e.parameter is MyClass)) { /* do something */ }
}

Method 2:

void MyBinding_Executed(object sender, ExecutedRoutedEventArgs e)
{
    if (e.parameter is MyClass) { } else { /* do something */ }
}

Method 3:

void MyBinding_Executed(object sender, ExecutedRoutedEventArgs e)
{
    var _Parameter = e.parameter as MyClass;
    if (_Parameter != null) { /* do something */ }
}

Method 4:

void MyBinding_Executed(object sender, ExecutedRoutedEventArgs e)
{
    var _Type = typeof(MyClass);
    switch (e.parameter.GetType())
    { 
      case _Type: /* do nothing */; break;
      default: /* do something */; break;
    }
}

[EDIT] Method 5:

void MyBinding_Executed(object sender, ExecutedRoutedEventArgs e)
{
    if ((e.parameter is MyClass) == false) { /* do something */ }
}

Which is the most straight-forward approach?

---

This is obviously a matter of personal opinion and style, so there's no right answer, but I think this is clearest:

void MyBinding_Executed(object sender, ExecutedRoutedEventArgs e)
{
    if ((e.parameter is MyClass) == false) { /* do something */ }
}

The == false is just more obvious than the !

---

I would go for 3 if you need the variable later or 1 if you don't need the variable. 2 is ugly because of the empty block.

However I think they all are straight-forward.

---

I would think just making an extension method would be a clear way of doing it:

public static bool CannotBeCastAs<T>(this object actual)
    where T: class
{
    return (actual as T == null);
}

You then simply make a check like so:

if(myObject.CannotBeCastAs<SomeClass>())
{

}

---

Methods 1 and 3 would be my picks, depending on what I actually wanted.

Method 1 "does something" if and only if the passed object is not of the expected type. This means the passed object could be null and still pass.

Method 3 "does something" if the passed object is not of the expected type, OR if the object is null. This is basically a one-pass check that you have a "valid" instance of the class to work with further.

So, whether I wanted 1 or 3 depends on what I was planning to do. Usually, when the variable isn't of the expected type or is null, I want to throw an exception. If I were happy with throwing just one type of exception (say just an ArgumentException), I'd use method 3. If I wanted to check for null separately and throw an ArgumentNullException, I'd use method 1 and add the null check.

Method 2 is functionally correct, but I'd rather invert the if condition as in Method 1, as an if block that does nothing is redundant.

I would never do Method 4. A switch statement taking the place of a simple if-else is unnecessary and confusing, especially in the manner you're using it.

---

To me, Method 1 is the most straight-forward, both on its own and by convention. This is the syntax I've seen the most if you just need to know if an object "is-a" certain class.

If you actually need to do something with the object "as-a" certain class, then Method 3 is the way to go.

---

Method 1 is the best in my view. It's very obvious what the code is doing and I can follow right along. Method 2 introduces unnecessary syntax that is easily corrected by Method 1. Method 3 requires me to think more than the other two (marginally, but still!), and it also uses extra space that isn't needed.

Remember code is written for people to read, and only after for machines to execute. Go with clarity every time.

---

If you want elegance and readability:

void MyBinding_Executed(object sender, ExecutedRoutedEventArgs e)
{
    bool isMyClass = e.parameter is MyClass;
    if (!isMyClass) // or isMyClass == false
    { 
        /* do something */ 
    }
}

I've always tried my best not to put too much logic in a single line of code, specially if conditions. I think the type check and negation operator might be annoying to parse on first glance.

---

Method #5 (a different spin)

public static class TypeExtensions
{
    public static bool IsNotTypeOf<T, X>(this T instance, X typeInstance)
    {
        return instance.GetType() != typeInstance.GetType();
    }
}

// ...

if(e.parameter.IsNotTypeOf(MyClass)) { /* do something */ } ;

---

I would be of the opinion that braced functionality should always match whatever brace pattern is in use in your application. For instance, in the case of iteration or conditional blocks, if you use:

If (foo != bar)
{
    //Do Something
}

well then this should be how you use brace patterned functionality at all times. One of my biggest bugbears with reading other peoples code (and this is especially true if they use CodeRush or Resharper) is the unnecessary terseness people add for no other reason than to display wizardry.

I am not saying the above is the best brace matching pattern however, use whatever one you feel comfortable with, what I would like to get across is that the pattern does not matter so much as the consistency of its use.

Personally, since C# is a terse language in comparison to, say VB.Net I would use long form statements or assignments (with the exception of var initialising) over more condense syntax to help aid later readability.

---

I like an approach used by one of the NUnit Assert's:

Assert.InstanceOf<MyType>(objectInstance);

BTW, If you have a set of checks whether object is of specific type like:

if(objectInstance is TypeA)
{
  // ...
}else
{
  if(objectInstance is TypeC)
  {
    // ...
  }
}

There should be some design issues like tied coupling between few types, so consider an other approach like injected map of associations or map like algorithm method per type

IDictionary<Type, Func<TParameter>>