For Each foolishness

Posted Mon, Jun 14 2004 17:12 by bill

Recently, Paul Vick posted about For Each in Vb.NET, claiming we should “thank them” for breaking the Strict semantics. Nothing could be further from the truth.  This was and is foolishness, a foolishness that removes the ability to catch errors at design time, and makes them only resolvable at run time. It breaks the very essence of what Option Strict is meant to indicate.

To understand the full implications let’s look at what really happens behind the scenes.

A For Each block, typically uses the collections Enumerator. So code such as :

   For Each obj As Object In myCollection
      …
   Next

roughly translates to:

      Dim obj As Object
      Dim enumerator As IEnumerator
      Try
         enumerator = myCollection.GetEnumerator
         While enumerator.MoveNext
            obj = enumerator.Current
         End While
      Finally
         If TypeOf enumerator Is IDisposable Then
            DirectCast(enumerator, IDisposable).Dispose()
         End If
      End Try

As shown in the above code, the enumerator’s Current property is the value assigned to the variable.  This is where the problem lays.

The problem is IEnumerator.Current returns the type Object, so all strong typing information is lost.  Given this limitation, it’s understandable that the VB team thought there was no real damage done by allowing implicit casts from object to the loop variable’s type.  Usually this kind of narrowing conversion would require explicit code if Option Strict is on.

Eg:

   For Each var As String In myStringCollection
      …
   Next

is allowed, even though Strict semantics would dictate it should be :

   For Each obj As Object In myStringCollection
        Dim var as String = DirectCast(obj,String)
      …
   Next

Sounds all nice and warm and cuddly hey? Well no, because what they decided to do has some serious implications.

First, the VB compiler in keeping with NON strict semantics, uses CType(obj, String) instead of DirectCast.  That means it will convert data such as Int32 to String as well.

Secondly, the above case of IEnumerator does NOT apply to arrays.  For arrays, the type is known, and IEnumarator is not needed. Hence for arrays, the code actually becomes:

    For i as Int32 = 0 to myarray.Length – 1
        Dim var As String = myarray(i)
        …
    Next i

This optimization is used in both C# and VB.ENT as it avoids having to go to type Object, and worrying about cleaning up the IEnumerator etc, etc.

Problem is here, VB.NET, due to what Paul thinks we should thank him for, allows implicit conversions, even when Strict is on.  So code such as :

      Dim myStrings(10) As String

      For Each var As Int32 In myStrings
         ..
      Next

compiles perfectly even with Strict On.  And given that a String may or may NOT convert to an integer, the above code may fail at runtime.  That is, we have lost the benefits of strict semantics.

In Whidbey, this issue becomes even worse as we now have Generics.  With Generics, there is an IEnumerator(Of T) interface, such that the Current property returns type T.
So a List(Of String) will actually have an IEnumerator(Of String) .  This means the problem we saw earlier of the enumerator only returning type Object no longer applies.

To the VB team’s credit the compiler does look for IEnumerator(Of T) and uses that if it is there rather than the older IEnumerator.  However, in keeping with the lack of Strict foolishness they think we should thank them for, the compiler lets you do unsafe implicit casts. So code such as :

      Dim myStrings As New List(Of String)

      For Each var As Int32 In myStrings
         ..
      Next

compiles perfectly and once again, the onus is shifted to runtime to detect the error.

This is BAD, BAD, BAD, BAD !!!!

If Paul wants us to thank him, he needs to fix it first.

If we look at the above cases, we have 3 distinct  scenarios:
(1) IEnumerable
(2) Arrays
(3) Generics IEnumerable(Of T)

In cases 2 and 3, clearly with Strict On, the compiler should NOT do narrowing conversions. The type information is preserved.
In case 1 however, it may be beneficial to have the compiler do those narrowing conversions for us, but this should be configurable. We should be able to set this to: no warning, warning, or error just like any other compiler setting.

If this requires two compiler switches, one for case 1 and another for cases 2 and 3, so be it.  But having Strict code semantics broken, and have design time errors shifted off into runtime errors is NOT good.

Oh, and just for the record, what C# does is pretty much as I have said VB.NET should do.  They allow an implicit narrowing cast (not conversion) in case 1, but in case 2 and 3 they preserve the type information and do not allowing implicit narrowing.

To put this simply, the VB.Net team has this one wrong.  It makes a complete mockery of Option Strict, and removes many of the type information benefits that generics brings to the table.  Shame on you Paul. 

Thank you Paul? NO WAY! You fix it first, and *then* I will be among the first to thank you!

 

 

 

 

 

Filed under: , ,

Comments

# re: For Each foolishness

Thursday, June 17, 2004 2:17 AM by bill

I used to be an "Option Strict" kind of guy back in the good old days of C++. I had an innnate belief in my compiler's ability to protect me from bad code. I worshipped at the altar of Bjarne's decision to favor compile time errors over runtime errors.

However, I have recently come out of the closet to reveal myself as an "Option Strict Off" kind of guy. There are lots of folks who write large systems using dynamically typed languages, and Option Strict Off is the closest that I can get to the semantics of those languages in the CLR world.

The catch, of course, is that you must write tests that verify the semantics of your code, instead of relying on the compiler to catch (arguably) syntactical problems (which IMHO static compile time type analysis really is).

Just my own personal story about how I've come to love VB (from a reformed C++ dev).

# re: For Each foolishness

Thursday, June 17, 2004 5:26 AM by bill

Hi John,

I think non strict code has its place, especially when it is dynamically generated code on the fly. And VB.NET does do that very well.
I still believe though, that you should not rely on implicit code if you are actually writing that code. Sure, use it Option Strict Off where you absolutely have to, or in cases where you might be retrieving member names from a database etc. But if you actually write the member name, then it means you knew at compile time exactly what you were calling but didn’t have it clearly defined anywhere. It’s at that point you should consider using interfaces and/or generated assemblies/code to do that layer of marshalling for you, as you then remove a wide range of possible *human* errors.
Most folk who know me well, know I am adamant about string literals in code being dangerous//evil. I have seen them be the cause of hard to locate errors time and time again. And I have seen that and experienced that when working with some of the best developers I know. A simple typo can end up taking hours to track down. And just as this is bad with string literals, when you turn Option Strict Off, you realistically subject all your code to those same flaws string literals have.

Getting back to the For Each issue however, if you want to use Option Strict Off, that’s fine, it probably won’t make any difference for you. But for people who do want to use Option Strict On, the current implementation breaks that. That is, at present we don’t have the *Option* of making that code strict.

Options are good :)

# re: For Each foolishness

Tuesday, June 22, 2004 9:26 AM by bill

I understand what you're saying here, but your statement about C# is wrong: VB and C# have the exact same semantics in regards to requiring casts in for each statements. The problem is that you picked a bad example: while VB has a conversion from String to Integer, C# has no conversion from String to int. So C# gives you an error because no conversion exists, not because you forgot to explicitly specify the cast. Change "List(Of String)" to "List(Of Double)", and you'll see that C# and VB have the exact same behavior.

# re: For Each foolishness

Wednesday, June 23, 2004 1:25 AM by bill

Good catch. The issue still remains that VB, unlike c#, does conversions to string and from string for numerics, dates etc. This is, in the for each, VB.NET uses Option Strict Off rules and semantics, regardless of whether option strict is on or not. This is bad. It means that errors in code that could be caught at compile time become difficult to test runtime errors.

And given the ability to strongly type with arrays and generics, there is no need for that injection of that Option Strict Off code.