C#: No, you don’t get to use the “as” operator any more | Hacker News Reader

C#: No, you don’t get to use the “as” operator any more

badcodemonkey.wordpress.com

22 pointsIndyan41 comments

Many language designers, including many on Microsoft's Managed Language teams, consider null reference and invalid cast exceptions to be huge blunders.

The "Nice" language shows that it's pretty easy to statically avoid this: http://nice.sourceforge.net/safety.html

The sad fact is that the Java and .NET base class libraries were designed before (1) generics and (2) a generic "Nullable" (aka "Option") type were available. Anyone designing a statically typed environment today should certainly include both and design a compiler to enforce their usage and safety.

... but since 2.0 .NET can do all the things listed in that Nice document. There are glaring gaps in the core libraries, which tick me off to no end, but that has nothing to do with the language.

Nice is a language, not a VM, so I'm guessing you're talking about C#.

In which case... uh... no. Of Nice's "advanced safety features" it only got generics (in 2.0) and internal iterators (in 3.0, via LINQ methods)

* It's a single-dispatch language

* Its instanceof (`is`) is the same shit as Java's (and the compiler is just as stupid)

* Object types are always nullable, it's not optional and it's not opt-out

* Its arrays are covariant (covariant arrays were added to the CLR for java compatibility, and C# got them from there because it was "free")

VM vs language: not really. The version of .NET heavily defines the capabilities of the language, as it's also tied to a compiler, and multiple languages tend to get new features at the same time. e.g.:

>Generics are only supported on version 2.0 and above of the Microsoft .NET framework, as well as version 2.0 of the compact framework. [http://msdn.microsoft.com/en-us/library/aa479866.aspx]

The day Microsoft (or anyone else) allows you to create generics in the VM for 1.1, which is quite a different one than 2.0 and above, I'll happily admit I'm wrong.

Single vs multiple dispatch: yeah, you can fake it with reflection, but it's not part of the language feature.

Nullable: Good point, missed that part of the page. That would be nice.

IEnumerables are covariant in .NET 4, I missed the covariant part too: http://msdn.microsoft.com/en-us/library/dd799517.aspx#Varian...

The main problem with non-nullable types is that the static analysis involved doesn't work for object fields., so you can't write code like this:

        if (object.some_field != null)
               object.some_field.foo();

because the compiler is unable to prove that it's correct. Instead you have to do a clumsy workaround like this:

    SomeType some_local;

    some_local = object.some_field;
    if (some_local != null)
        some_local.foo();

> The main problem with non-nullable types is that the static analysis involved doesn't work for object fields., so you can't write code like this

You shouldn't get to write code like that anyway. It's C. It's crap.

Here's what you write:

    object.some_field.notNull(f => f.foo())

and that's only valid if some_field is of type Nullable<whatever>.

If you have to check it for null then, by definition, some_field here isn't a nullable type. If it was, you'd just do:

    object.some_field.foo();

Am I missing something here?

C# (values) or Nice don't use a full, explicit nullable generic type, and instead tag the original type (`int?` for C#, `?String` for Nice).

This means some_field could be a type tagged as nullable (e.g. `some_field` would be defined as ?Foo).

An alternative would be `@NotNull` opt-in anotations, where `some_field` would not be tagged, therefore nullable, therefore could hold the value `null`.

Thus, if you have to check it for null then `some_field` is a nullable reference, in a language which may have nullable and non-nullable references.

That seems like it would be a trivial thing to do in an early compilation step... why is the workaround necessary?

The problem is that if the code looks like this:

        if (object.some_field != null)  {
               some_unrelated_call();
               object.some_field.foo();
        }

proving that some_unrelated_call() didn't set object.some_field to null is very difficult.

In general, it's also difficult to prove that 'object' doesn't get set to null too.

Depends on what you mean by "general", I suppose, but if 'object' is a local variable in a Java-ish language, then there is a fairly straightforward data flow analysis that can do a good conservative estimate.

but if 'object' is a local variable in a Java-ish language, then there is a fairly straightforward data flow analysis that can do a good

But if it's not, and/or if it can be adress taken...

1. properties mean field invocations are not referentially transparent (you can only mitigate this by removing properties, or having referential transparency as part of the type)

2. multithreading means the value of the field can change from under you between the check and the access (you can solve this by eliminating threads or by transactioning your accesses)

I'm pretty sure point 2 is incorrect. The compiler doesn't worry about multithreaded access to properties/variables when optimizing.

You're right that such access can cause problems though. If you do want the compiler to worry about multithreaded access when it compiles, you can use the `volatile` keyword.

> I'm pretty sure point 2 is incorrect. The compiler doesn't worry about multithreaded access to properties/variables when optimizing.

This is not about optimizing, it's about static verifications. And that's very much the point: you can't statically verify this piece of knowledge if the code can be used in a multithreaded context.

Nonnullable (static) class fields and constructors don't mix however, which is why you don't see it as a language addition so far.

Nonnullable local variables and function parameters however could easily work within the current c# language.

Yes, null refs are bad.

But so is an InvalidCastException -- Which is what your new code could throw.

I guess the point of the article is that when you're doing an invalid cast, you in fact want to get an InvalidCastException, not NullPointerException.

But I do the null check all the time, and then skip the "Save", but have logic in the 'else' to do something like create an actual customer account.

The problem with the solution provided is that you get an exception, whether you can deal with it in the body or not. And unlike Lisp, you're going to unwind the stack at that point (or be in the catch). All in all, not happy with the proposed solution.

In your case, then, using "as" is the right thing to do.

Actually you don't want to ever use exception driven development. The use of the as operator is a simple metadata check. Throwing exceptions in .Net is expensive.

Just because I felt the need to check my own facts (Friday afternoon?) I wrote a simple console app with two classes Class1 and Class2 : Class1. Using a Stopwatch object I checked the elapsed ticks for an 'as' an 'is' and an invalid cast exception check.

'as' = 5 ticks

'is' = 4 ticks

'invalid cast' = 147990 ticks

Note this was just enough test to validate my assumption and that I never condone my developers taking an optimize first development approach... But there is still a right and a wrong way to do the job sometimes and exception driven development is not the right way.

> Actually you don't want to ever use exception driven development. The use of the as operator is a simple metadata check. Throwing exceptions in .Net is expensive.

it doesn't matter, it's throwing an exception when the caller code is broken. That's ok, it's an error.

I'm not sure what you mean by exception driven development.

The original code was trying to express that e.NewItem should be a Customer. In all the following code, it is still an error for e.NewItem to not be a Customer. The question is what should happen when that error occurs.

There are three options discussed: 1) NullReferenceException, 2) Silent error, 3) InvalidCastException. Performance does not come into it, since if any of the error handling code paths get triggered, there is a bug which needs to be fixed. The claim in the article is that by throwing the right exception (InvalidCastException) it will be easier to track down the bug than if the wrong exception was thrown, or if the error was ignored.

Some clarification seems to be required since I am being voted into oblivion (perhaps never ever use ever or never?:) )

When writing some code that relies on a method in a specific class or interface being present then actively test for it and react appropriately.

Writing the code based on how it is hoped the program state will be and letting the exceptions fall out is what I mean by exception driven development. In many cases this is a code smell that indicates a section that was not completely thought out.

The next step with an invalid cast exception or a type check is to take action and do something about it. When faced with an exception there can be more ambiguity about it's source especially when there are many lines in the try block or if you are calling into other libraries.

What if there is some code in the save method that throws the exception for type casting and the invalid cast exception assumes that the source is the base class problem? Now you can add further debug time to the cost of trying to short circuit the type check that was intended up front.

Unfortunately, the real problem here is a broken API. A statically typed language whose standard library includes APIs that require downcasting is pretty embarrassing.

In general though, if the situation is truly exceptional (wrong type being passed) then counting the ticks used throwing the exception should be compared against the ongoing cost of the increased complexity to the codebase to handle the exception in another fashion (e.g. an if/else block).

Also, if you're just comparing the ticks of as+is VS invalid cast wouldn't you want to multiply each by the # of times they'll be executed? The as/is would be executed EVERY time the function in question was called. The invalid cast would, in this case, likely be executed once or twice ever and then the bug that is causing an InvalidCastException would be fixed.

That's part of it. But the author should articulate it.

There is also certain (specific) situations where the original code (e.NewItem as Customer).Save() would fail but ((Customer)e.NewItem).Save() wouldn't. That specific case is where the type of e.NewItem has an implicit cast to Customer (as would return null, but a (Customer) cast would work).

But if you're doing implicit casting you might be facing other issues.

Yeah I no argument that it's good to know what your code does but what you choose to do is context specific, why would throwing InvalidCastException be better than NullRef, neither tell me why the failure happened.

On a somewhat unrelated note why is this trivial C# 101 lesson on the bloody front-page. I've got to imagine maybe 1 out of 10 readers care and far fewer didn't already know this.

InvalidCastException include the class name, doesn't it? That makes it order of magnitude easier to track down where it came from. The NullRef just tells you that it MAY have been that the 'as' failed, or it could have been a bona-fide NullRef.

It's best if you think about the reason that "as" was added to C#. It's so you can do this:

  Customer c = person as Customer;
  if (c != null)
  {
    // do something
  }
  else
  {
    // not a customer.  must be just a regular person. do something else
  }

instead of this:

  if (person is Customer)
  {
    Customer c = (Customer)person;
    // do something
  }
  else
  {
    // not a customer.  must be just a regular person. do something else
  }

... and that's it. It's just somebody's idea of a little cleaner syntax, not having to cast twice to get where you want to be.

In that light, the author's second example is closest to correct. It's just missing the "else" bit to handle the case where the base object you get passed is not be the exact type you're looking for.

EDIT: Actually, yes, ahem, I see - if e.NewItem is actually null you're going to get confused between what the problem actually is here, and that might not be repro. Perhaps I posted a little too soon... but assuming you check for that separately, the point stands :)

I have to disagree here - I prefer the initial code example. It's cleaner, and as soon as you get to the line of code in the debugger you'll know exactly what's up regardless of the on-the-surface-misleading null exception. So what if it's initially confusing?

In any case both examples are wrong, and I don't think that's made clear enough - you should be actively looking to check for exceptions, not passively accepting that they might happen.

As an aside, I've always found it cleaner to say:-

    var cust = e.NewItem as Customer;
    if(cust == null)
        throw new BlahException("blah blah blah");

    cust.Save();

Compared to, say:-

    if(!(e.NewItem is Customer))
        throw new BlahException("blah blah blah");

    var cust = (Customer)e.NewItem;
    cust.Save();

(Note that in the second case that a thread could jump in and spoil your day anyway, not to mention that .NewItem could be some crazy property which randomly decides to return null in the second case, and you'd get the same old nasty exception, as mentioned in other posts)

So I think the 'as' operator has value in this alone as handy sugar.

Obviously the real problem is that null exceptions happen at all, or that anything actually returns null, and by God is the Option discriminated union in F# a joy (union of Some/None) after dealing with all that shizzle.

I would make the public version of Save a static method on an abstract class (or a static method in a utility class) and avoid the whole worry over null. It's only a bit more work up front.

    abstract class Foo
    {
        public static Result Save(Foo foo)
        {
            if(foo != null)
               foo._Save();
            else
            {
                //Do Stuff...
            }

        }

        protected abstract Result _Save();
    }

Then use like so,

    foreach(Foo s in xs)
    {
        Foo.Save(s); //or s.Save() if Save() is an extension method.
    }

This misses the author's point. In his example the bug you want to detect is that the referenced object in question is not a Customer (or a Foo in your example).

In your else block you can't report anymore information than the NullPointerException in the author's original example reported.

If you still use an 'as' operator at the callsite then you've lost all useful information. If you do a cast then you'll at least get an exception with WHAT it was you were trying to cast.

I don’t think that I’ve missed the author's point; but any time I see an if statement and polymorphic code used near each other, I question whether or not the programmer understands OOP. His example is not exactly using polymorphic code, but he is basically dispatching based on the runtime type.

We know that a method that accepts parameters that are reference types should check to see if they are null. Since instance methods cannot check their first parameter (the this pointer), we make it impossible for outside code to call instance methods. Now callers never have to worry about the runtime type of an object or whether or not it’s null.

If you’re going to circumvent the type system with explicit casts then, of course, you’re going to get invalid cast exceptions, but that’s for the client code to deal with.

Or use an interface, then anyone making changes further down the line could leave the 'Save()' calling code alone entirely.

That would work too but an abstract class let's you keep the _save method private so that client code can't possibly call that instance method on null.

Personally I think that now that we have extension methods, all public methods should be extension methods on interfaces or abstract classes. This basically gives you something like the pattern matching in ML and Haskell.

Actually... he misses a big use case (stolen from haskell)... Using extension methods (which accept null receivers) and "as" you can get pretty readable straight line code... something like:

    name = (obj as Customer).LiftName() ?? "Guy Incognito"

Interestingly, built in support for this (w/o the need to write a "LiftName" extension method) almost made it into VB 2008, but it ended up getting cut.

There are subtleties to it too - http://blogs.msdn.com/b/ericlippert/archive/2009/10/08/what-...

:-)

I tend to use it because it makes it reading from left to right feel right instead of jumping back and forth.

And really, don't give me null objects unless I specifically say I can handle them.

((e.NewItem as Customer) ?? Customer.NullObject).Save();

Edit: The idea being, you have a sub-class of Customer which implements the Null Object pattern, which you instantiate through a static member on Customer or a Property method or what-have-you. The great thing about this is that you can tailor the behavior in this situation to whatever you want. Want to throw an exception? Sure, throw whatever exception you want from CustomerNullObject.Save(). Want to log an error and otherwise silently fail? No prob, easy peasy.

strange... ecma script allows it...

Reply on news.ycombinator.com