CodeSOD: The Properties of Contract Development

James's management had more work than they had staffing for, so they did what any company would do in that situation: expand their staff. No, of course not, I'm kidding. They bundled up a pile of work and shipped it off to the contractor who gave them the lowest bid, provided absolutely no oversight or code-quality standards in the contract, and hoped for the best.

What they got back was a gigantic pile of code that compiled. That is the only positive thing one can say about the code, because it certainly didn't work. Within a few weeks of starting reviewing the gigantic pile of garbage the contractors turned in, the dev team reached the decision that it would be quicker to rewrite from scratch than it was to try and pick apart the trashpile and reshaped the refuse into something approaching their actual requirements.

James sends us just one small VB .Net property definition which summarizes the kinds of problems they were dealing with:

Public Overrides ReadOnly Property id as Object
		Return id
	End Get
End Property

A quick primer on .NET's property architecture: it's syntactic sugar for generating getters-and-setters, but allowing the property to behave as if it's just a variable. So if a class has a property called foo, you can do things like x = or = x. It's actually calling getter and setter methods, but it's invisible at the point of use.

I bring this up, because on the line where we Return id, we're calling the getter for a property called id. That's this property. So we have an infinite recursion and never actually get a value. Which we couldn't have a value in the first place, since this property is ReadOnly- it only supports Gets. Nothing about this works. Nothing about this could work.

And even if it did work, note how we cast to Object, which is the VB .Net way of saying "I hate worrying about types, so I'll just cast everything to object and pretend that types don't exist."

And that's the basic philosophy underpinning all the other code in the application: it's written in ways that guarantee that it could never work as intended. Frequently, it's simple misuses of the language, like this one.

The "correct" way to implement this, by the way, would be:

Private _id as Integer
Public Overrides ReadOnly Property id as Integer
		return _id
	End Get
End Property

Someplace else, probably in the constructor, we'd set _id.

This is a pretty easy, beginner mistake to make in .NET. Hell, I've had the brain fart and written code not too dissimilar from the original example. But I never checked it in. I certainly never handed it off at the end of a contract and claimed that I fulfilled the requirements.

The real WTF isn't the code: it's the process that let people hand this code off as if it were a completed set of requirements.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

This post originally appeared on The Daily WTF.

Leave a Reply

Your email address will not be published. Required fields are marked *