I HATE OLD VB!
A pet-peeve of mine is when I work on code that written with legacy VB6-style methods and functions rather than the more current .NET versions.
Is there a huge performance penalty between calling Len() on a string versus .Length and that sort of thing? Not really. I mean, I’m sure there probably is on some level, but nothing a user is likely to notice in an average application.
My big gripe is that by forcing yourself to use what’s in the framework only, you can sometimes find better ways of doing things.
The best example for that I can give is the use of IsNumeric(). In applications I’ve worked on, I’ve often seen code where the validation of a textbox (for the input of an order number, for instance) uses IsNumeric to check to see if the number is valid and then will blindly set the value of a variable, parameter, etc. to it that is an Integer, a Long, or whatever.
For this example, let’s assume that the order number field is an Integer.
Here are a few inputs that pass the IsNumeric() test:
5
-5
+5
$5
5.5
2147483648
The first three are mathematically valid, so let’s ignore those for now. I rather not accept leading + or – characters unless we are working with values that can legitimately be negative or positive.
The last three items on the list are a bit more interesting, though.
In strict terms, “$5” isn’t a valid number. If we explicitly cast it to an integer with CInt(), it’s smart enough to trim off the dollar-sign, but is that what you want? Call me old-fashioned, but I like predictability… If I have a field that only is meant for numbers, it shouldn’t work if I throw in something that isn’t a number.
With “5.5”, it’s clearly not an integer. If the code simply checks the input against IsNumeric() and then runs CInt(), it will round the value up to 6. Assuming it was a legitimate typo, if the user attempted to search for order 55 and got back order 6, it might lead to confusion. It’s better to err on the side of caution up front, having them fix the input before searching again.
Lastly, there’s a search for “2147483648”, which is one digit higher than the maximum size an integer data-type can be. It will pass the IsNumeric() test, but will cause an overflow when CInt() is attempted. Not good. Unless you have pretty robust exception-handling (and I don’t mean just tossing everything into a Try/Catch block), this can either result in unexpected behaviors within the app or, more often, user-facing errors that aren’t easy for them to understand.
The alternative solution to the above scenario is to use Integer.TryParse() With that method, you’re not only evaluating the string value to see whether it can be converted to an integer, but (assuming it can be converted and you passed in a variable as the second parameter instead of Nothing) it also does the conversion for you. With the above list of inputs, the only ones that pass the Integer.TryParse() method are the first three. And, again, if I wanted to be even more picky, I could reject the + or – prefixed input for maximum consistency.
Regular-expressions might be good for an initial validation pass, I guess, but that’s a whole other topic…
Ok, great, but so what…?
What does this all have to do with C#? Very little… I just wanted to explain my motivations for wanting to prevent some of this stuff from finding its way into new code and to identify legacy implementations that probably need cleaned up. It’s not just IsNumeric() that I have an issue with, it’s also IsDate(), which can be easily handled through Date.TryParse, UCase, Len, Space, Left, Right, Mid, InStr, MsgBox, Format, Replace, LikeString, etc., etc., etc.
I needed to identify where in the code these were being called, but it should be done in such a way as to allow me to make exceptions when needed and to differentiate between the Good, Bad, and the Ugly of VB calls…
For that, I needed FxCop…
That is where the C# stuff came in.
Aside from there not seeming to be any (at least not useful) official documentation on FxCop, I had to rely on third-party examples. Once I got over the initial issues of using the newest version of FxCop with samples code made for old versions of it, I eventually got the gist of what was being done.
To make a custom FxCop rule required me to make a C# library DLL with the various introspection rules. It also needed an XML file saved as an embedded resource, which wasn’t too involved.
Really, the hardest part of writing the custom rules was figuring out how the introspection was evaluating the code and, more importantly, what I needed to change in order for it to evaluate what I wanted.
My goal was simple enough. For starters, just show everything that referenced the Microsoft.VisualBasic namespace. Once I got that working, I’d figure out how to narrow it down just to the stuff that I wanted to no longer use in new code.
I’m still tweaking it a bit, but I finally got it working (mostly) the way I want. I go within the MethodCall and examine the Method’s DeclaringType.FullName… If it starts with “Microsoft.VisualBasic”, I know I’m on the right track… There are a couple namespaces that I exclude right off the bat (like “Microsoft.VisualBasic.CompilerServices.Conversions” and “Microsoft.VisualBasic.CompilerServices.ProjectData”). Otherwise, I call a custom routine of mine that categorizes the call into a few different rule categorizes I came up with.
To categorize the call, I use a combination of the method’s namespace as well as GetUnmangledNameWithoutTypeParameters(true)
Everything In Its Place
I came up with four categorizes:
Unknown -- Anything that isn't explicitly assigned to one of the following three categories by this routine
Allowed -- Anything that is something that there isn't a good alternative for in the .NET framework or isn't worth tackling just yet
LegacyVB -- Methods and routines that there are better .NET alternatives to (e.g. Len, MsgBox, InStr, Replace, UCase, etc.)
ConsiderAlternative -- Methods and routines that might be better suited for native .NET methodology (e.g. DateAdd, IsNumeric, IsDate, etc.)
This seemed to do the trick. I could now analyze any project I was working with an see how many calls were using “the old way”
Ultimately, it came down being more of a conversation starter than anything else.
Some calls, like with those to MsgBox, were brought over from older code, while all of the new code was written using the .NET Framework alternatives. Other times, there were calls even in the new code being made because no one had realized there were other (better?) ways of doing the same thing.
For more, the call I used a lot, but hadn’t realized there was an alternative to was IsNothing() I’ve slowly been trying to break my habit, if for no other reason than it’s so much easier to read “If SomeValue Is Not Nothing” rather than “If Not IsNothing(SomeValue)”. It’s a subtle difference when writing it, but more of a value when it’s being read (especially by someone else).
Ultimately…
I enjoyed learning and working with C# a bit. I still think case-sensitivity is retarded, but there’s a lot about C# that I started to get used to really quickly.
I don’t know, I’ll probably do a few more small projects with it and see how it goes.
