Static Code Analysis and you

There are a lot of ways of improving your code quality: Pair programming, code reviews, unit testing, just to name a few. Static code analysis is one of them. While good code analysis tools can cost thousands of euros and will highlight issues in your code you’d have most probably missed otherwise, it is important to understand why these issues can lead to subtle bugs.

I’d like to shed light on some the issues Resharper points out from time to time, and why taking theses issues seriously can help you to write better C# code.

Possible compare of value type with ‘null’

The following code snipped is taken from our A* pathfinding implementation and has led to the R# issue Possible compare of value type with ‘null’:

Strictly speaking, the type T could be a struct that implements the IAStarNode interface. For structs (and other value types), this comparison will always fail, which might be as intended. The clean solution would be to compare to the default value of the specified type:

Assignment to a property of a readonly field can be useless

This can be a tricky (and acutally dangerous) one. Consider the following code snippet:

Resharper will complain about the assignment of the value 42 with Assignment to a property of a readonly field can be useless. Field type is not known to be reference type.

The problem is that readonly fields can only be initialized in the constructor. As GenericClass doesn’t do so, its field will always have its default value. For reference types, this is null, and sooner or later you’ll run into a NullReferenceException – but you’ll find the issue at least. For value types, the result is a little more subtle: In this example, the field will be initialized to the default value of our struct, having its Value property set to 0. While you are absolutely sure of having assigned it the value of 42, this did never happen.

Non-readonly field referenced in GetHashCode

Let’s say you’re working on your own vector struct. At some point, you start writing unit tests, which always fail – you forgot to override Object.Equals. Doing so, the compiler will complain about having overridden Equals, but not GetHashCode.

So you provide your own implementation of GetHashCode:

Now, Resharper will complain about a “Non-readonly field referenced in GetHashCode”. Eric Lippert has written a long and detailed explanation of why this might become a problem for you.

Basically, the GetHashCode method is used whenever you’re putting an object in a hash table.  Hash tables store their items in buckets, trying to achieve an equal number of items in each bucket for faster lookups. They are using the hash codes of these items for indexing into the correct bucket.

Now, let’s say you put your vector into a hash table, for any reason. If you change the values of your vector now, its hash code will mutate, and the containing hash table will fail to ever look it up again: If the hash code of your vector was 42 when it was added to the table, then it was put into the bucket with the index 42. Modifying the vector could make the hash code change to whatever value, say 73 for instance. If you’d want to check whether the table contains your vector now, it would try to look it up in bucket 73, and fail to find it.

Author: npruehs

Nick Pruehs is a Co-Founder of Slash Games, Hamburg. In 2009, he graduated as “Best Bachelor” in computer science at Kiel University. Two years later Nick finished his master’s degree in “Sound, Vision, Games” at Hamburg University of Applied Sciences, becoming the Lead Programmer of Daedalic Entertainment shortly after.

Questions? Comments? Suggestions? Your turn! :)