Friday, February 17, 2006

Upgrade Fun - Moving to VS2005

Following on from my first post, we are now starting to upgrade for real and trying to get rid of those warnings. This is being hampered by the background compiler: every time I fix up a bit of code off it goes, hogging my CPU, until it has gone through the entire solution I guess. A search on the MSDN forums turns up this from MS guy Matthew Gertz:

"The background compiler cannot be turned off in Visual Basic .Net – it’s actually the thing that’s providing all of the Intellisense information, formatting information, and so on. I discuss this in some detail here. In that article are some tips to improve performance in larger-scale applications."

To minimize this problem, I am opening our projects one by one (bottom-up in the reference tree) instead of all at once in our monster solution. In fact, I think our development is going to have to continue this way until the background compiler's performance (or at least its perceived performance) is greatly improved.

Using an instance variable to access a shared member (Error ID: BC42025)

I would like to turn this warning off rather than fix it, because using a short
variable name in place of a sometimes quite long class name can reduce clutter and
make the code easier to read. However one of the ways we do this is we declare an
instance of an enum type (nested in another class) to provide short-cut access to
the enum literals, which results in an "unused variable" warning which I definitely
don't want to turn off.

Fortunately, initializing the enum variable removed the "unused" warning, at least in this compiler version. This is fortunate because the warning was also occurring when accessing members of the DialogResult enumeration on a form: Form also has a DialogResult property which hides the enum so you end up having to fully qualify it (or partially qualify it if you import a parent namespace, e.g. System.Windows).

Getting out of bad VB6 habits, and other details

The new compiler is much more fussy (a Good Thing) at pointing out poor form, including:

  • Not explictly initializing reference variables if they are going to be used before an assignment;

  • Not explictly returning Nothing from a function (a variant of this is having return statements inside a Select - you're then forced to define an "else" clause, which is another Good Thing);

  • Leaving unused variable declarations lying around.

Mind you, the explicit initializer thing is a bit of a pain when you're only going to initialize it to Nothing anyway. I've gotten too used to seeing lack of an initializer as an invisible Nothing or 0 (note the compiler doesn't fuss about default initialization of value types - why the inconsistency?) Also, the parser doesn't recognize patterns such as this, and reports that ref is used before being initialized:

Dim isFirstTime As Boolean = True
Dim ref As SomeClass
For i As Integer = 0 To 10
If isFirstTime Then
ref = SomeMethod()
isFirstTime = False
'use ref
End If

Another annoying variant of this is "Variable 'XXX' is passed by reference before it has been assigned a value. A null reference exception could result at runtime." This is annoying when you are passing by reference because the method is going to initialize the variable. This is a weakness of ByRef in VB: it means "in/out" and we have no keyword to specify just "out".

Proper XML commenting - Hooray!

We had had a half-hearted attempt to add these in using the VBCommenter add-in in Visual Studio 2003, but stopped when we found Intellisense wasn't picking up our descriptions. We've now got to fix syntax errors (invalid whitespace), mismatched param elements etc. in the existing comments.

I am getting a strange error though: "XML documentation parse error: "Whitespace is not allowed at this location. XML comment will be ignored." This is being reported inside the summary element. I.e. I can cut all the lines between <summary> and </summary> and it is fine, but when I paste the original text back (which contains no XML or anything obviously funky), the error springs back.

D'oh - the summary text contained an ampersand! So it seems like it is a real XML document fragment.

Inappropriate use of 'Overloads' keyword in a module

This surprised me. The help link merely says that modules aren't classes and therefore can't use MustInherit etc., blah, blah. Nothing about why Overloads is invalid on Module members. You can use it on Shared class members, so what's the problem? Amanda Silver explains:

We decided to give this warning because the application of the Overloads keyword only makes a difference to the semantics of the application when the method is overloaded across an inheritance chain. As the Overloads is superfluous in the context of a Module (and possibly misleading) we decided to emit a warning. Note, however, that the warning can be turned off by going to the project properties and selecting the "Disable all warnings" check box.

I love the last sentence!

I need to revise the purpose of Overloads and Shadows to fully understand this, but I would have thought that if you get a warning for Module members, you should get one for Shared class members too. Stay tuned.

Name '_blah' is not CLS-compliant

In a number of places, we have protected fields which, because they are fields, begin with an underscore. Because protected members are exposed outside the assembly and therefore available to other assemblies, they should be CLS-compliant (which means they can't begin with an underscore). The easiest way round this, since we don't need to interop with other .NET languages, is to mark the entire assembly as not CLS compliant in Assembly.vb.


Daniel Moth said...

My advice is to simply turn off the half-baked "Use of variable prior to assignment" unless you can live with loads of false positives:
(links in 6th paragraph)

Ian said...

I actually did spend the time (about a day and a half) to go through and add initializers to clear these and other errors and warnings, and overall I'd agree with you: I should have just disabled the warning.

There was one situation I remember where the warning uncovered a slightly dodgy situation, i.e. no "else" clause on a case, which showed up where a variable was initialized in each arm of the select. However since 90% of the time the programmer will remove the warning simply by initing the variable to Nothing rather than complete the case, it's probably not worth worrying about.

Kevin Wilson said...

To get rid of the error "Inappropriate use of 'Overloads' keyword in a module", simply remove the "Overloads" keyword from the function declaration and everything compiles without errors and you get the same functionality. In VB.NET 2005 when you have 2 declare statements that have the same name, but different paramteters/returns... "Overloads" is assumed and it treats them as such.

VB.NET 2003 and earlier require the keyword "Overloads."

fredd said...

Ian... bloody brilliant. Thanks for the tips about the "else" requirement and the CLS compliancy...

Kelowna Kid

Dave said...

I am a bit confused on this one

If var Is Nothing Then
End If

Gives a "Use of variable prior to assignment" warning?! Coming from a c/c++ background I assumed this type of syntax was synonomous to checking if a pointer was null? Could someone clarify this?

Manolito said...

Tnx for that Kev. I'm kinda new to .NET and I'm using 2008, that helped a lot in my discovery.