CodeSOD: Serial Properties

Jan wrote some code that set a property, and a few lines later had to write code to read that value- and the compiler complained. Which is what drew his attention to this C# code:

public string ViewNodeFilter
{
protected get
{
if (viewNodeFilter.IsNotValid())
{
return “null”;
}
return new JavaScriptSerializer().Serialize(viewNodeFilter);
}
set { viewNodeFilter = value; }
}

Now, one of the features of properties in C# is that the getter and setter can have different access levels, which we see here. It’s odd and unexpected, and when we look at the implementation, we can see why the getter is protected: it’s not a getter, it’s a JSON serializer.

But there’s a lot more oddness in here. First, the property is a string, so when we serialize it… we’re just serializing a string. Then there’s also the IsNotValid method, which is not part of string, which implies that it’s an extension method. Extension methods are a C# bit of syntactic sugar that allows you to write a function which accepts an object as a parameter (in this case, a string), but invoke the method as if it were a member function of the object. They can be powerful and useful, but this is peak “not how you use this”- every string gets a IsNotValid() method, this way, which is likely not what we want.

This is a surprisingly common problem in the .NET languages, though. Since you can attach code to getters and setters, but access looks just like an assignment expression, people put all sorts of surprising code in there. Would you expect foo = viewNodeFilterHolder.ViewNodeFilter to serialize to JSON? I wouldn’t. But since the data is a string, does it matter? Well, it does when I get surprised by the string “null”.

All in all, this is an ugly little booby trap, that represents a pattern common in Jan’s application.

Source: Read More