Tuesday, May 31, 2011

Virtual properties - they're just dumb

Have you ever seen code of the form:

// Make it virtual so that implementers can provide their own version for them to use
public virtual IManager Manager { get; set; }

?

Code like this just angers me. For one, why didn't they pass it into the constructor? This is much clearer. For two, why does a *base* class need to define what classes the derived classes need to use in order to accomplish their tasks? This is ludicrous. That particular property isn't even used in the base class anywhere - and that's the main problem with it.

When you put something like this on your base classes, it's implying to the deriver that there's some reason that the base class needs it, so the deriver should provide their own implementation that doesn't break the invariants that the IManager interface is bound to. However, since it's not actually used in the base class (notice the comment? It very clearly states that the implementers will be required to use the class, not the base class), the base class shouldn't even have a reference to the IManager type.

The codebase that I was looking at was a real joke, and the original implementers had no idea what OOP means. Without any real depth of knowledge, OOP can quickly turn into POOP.

1 comment:

  1. I've absolutely been guilty of this in the past, and it is pretty indefensible. It's usually caused by coder confusion between abstraction and intent. To whit: "Well, this general Message class is meant to be inherited, so I'll leave it abstract. But every Message will need SOME kind of destination address, right? I'll just leave this virtual property here..."

    This doesn't excuse the practice, of course. You're perfectly right that it's a terrible design, and you do a good job of explaining why. I just think it's useful to be aware of the situations in which the temptation to use the anti-pattern might arise.

    ReplyDelete