# Wednesday, October 11, 2006

One of the things I do for many of my clients is code review. This comes in two flavours. The first is where we all pile into a room with a projector and someone walks us through the code while we ask questions. Sometimes as a result of this the original author has to go away and rewrite bits of the code, but it's just as likely that the outcomes will be everyone understanding how something works, or knowing what there is in someone else's part of the application. The second is less interactive. I read over code and point out bad, dangerous, sloppy, or hard to maintain code. This might be hardcoded error messages in a multilingual application, or non parameterized dynamic SQL, or poor object orientation, or any one of hundreds of other things. Often I point it out not to the author of the code directly, but rather to the author's manager. Or in at least one case, their former manager. (As in, now that we got rid of this person, can you tell just how much mess he left behind?)

Now, if you are thinking of going to Team Systems, let me give you another reason to do so. It can automate a lot of these types of checks. For example, here's a really quite poor little class:

Public Class Company
    Private networth As Integer = 0
    Public name As String
    Private foundingdate As Date

    Public Sub New()

    End Sub

End Class

When I run static analysis against this, in no time flat it points out:

Running Code Analysis...
C:\Program Files\Microsoft Visual Studio 8\Team Tools\Static Analysis Tools\FxCop\FxCopCmd.exe /o:"bin\Debug\Information.dll.CodeAnalysisLog.xml" /f:"bin\Debug\Information.dll" /d:"C:\WINDOWS\Microsoft.NET\Framework\v2.0.50727" /r:C:\Program Files\Microsoft Visual Studio 8\Team Tools\Static Analysis Tools\FxCop\\rules
MSBUILD : warning : CA1020 : Microsoft.Design : Consider merging the types defined in 'Information' with another namespace.
MSBUILD : warning : CA2209 : Microsoft.Usage : No valid permission requests were found for assembly 'Information'. You should always specify the minimum security permissions using SecurityAction.RequestMinimum.
MSBUILD : warning : CA2210 : Microsoft.Design : Sign 'Information' with a strong name key.
MSBUILD : warning : CA1014 : Microsoft.Design : 'Information' should be marked with CLSCompliantAttribute and its value should be true.
C:\Documents and Settings\Administrator\My Documents\Visual Studio 2005\Projects\Information\Information\Class1.vb(6): warning : CA1805 : Microsoft.Performance : Company.New() initializes field networth of type System.Int32 to 0. Remove this initialization as it will be done automatically by the runtime.
MSBUILD : warning : CA1823 : Microsoft.Performance : It appears that field 'Company.foundingdate' is never used or is only ever assigned to. Use this field or remove it.
MSBUILD : warning : CA1051 : Microsoft.Design : Make 'name' private or internal (Friend in VB, public private in C++) and provide a public or protected property to access it.
MSBUILD : warning : CA1823 : Microsoft.Performance : It appears that field 'Company.networth' is never used or is only ever assigned to. Use this field or remove it.
Code Analysis Complete -- 0 error(s), 8 warning(s)
Done building project "Information.vbproj".
========== Build: 1 succeeded or up-to-date, 0 failed, 0 skipped ==========

Meaningless initializations, unused variables, public member variables... they get found. It's a great start.


Wednesday, October 11, 2006 9:33:04 AM (Eastern Daylight Time, UTC-04:00)  #