4

Sometimes I override methods in base classes. Sometimes I even override them with an empty method, because what I want is to prevent the behavior.

In the past I would write something like this to show the intent of bypassing the base method:

protected override void OnMouseUp(MouseEventArgs e)
{
    // base.OnMouseUp(e);
}

(I know a commented line of code is a bad thing. I used to do it)

But I want to do better:

  • How do I document the intention of the override? specifically:
  • What do I write in the override's XML (<summary>?) documentation?
Camilo Martin
  • 37,236
  • 20
  • 111
  • 154
  • 1
    http://en.wikipedia.org/wiki/Liskov_substitution_principle – Eva Feb 12 '13 at 17:14
  • @Eva I'm not sure of what you mean. From a quick read of the article, I take it you're saying I shouldn't prevent base class behavior? – Camilo Martin Feb 13 '13 at 07:29
  • [Here's](http://stackoverflow.com/questions/56860/what-is-the-liskov-substitution-principle) a SO question that explains it better. Basically your subclass should not violate the invariant of your superclass. Your subclass should add functionality, not change or remove functionality. Usually if your subclass violates LSP, you're probably looking at a composition situation instead. – Eva Feb 13 '13 at 15:49
  • @Eva I see. If I recall correctly though, the times I've used this was to modify drawing behavior and such things; there weren't "side-effects". If a method "doesn't have side effects", in the sense that those aren't tangible to the code, modifying the behavior of base classes is ok, right? – Camilo Martin Feb 14 '13 at 06:30
  • It's not about modifying the state. It's about being able to use the subclass as a replacement (or substitute) for the superclass in all instances. In the links I posted, there's the example of the Square and the Rectangle. You can implement a Square subclass of a Rectangle without side-effects (there's an example in one of the answers in the second link) but it still violates LSP because one of the invariants of a Rectangle is that when you change the width the height should not change even if the width and height are different. It's explained better in the second link. – Eva Feb 14 '13 at 06:43
  • @Eva I see. So the rule of thumb is to think "would a user of this subclass be able to use it as if it was the base class?" when overriding base methods. In this case I think it boils down to spec right? Because, one could just not say that a rectangle requires width and height to be independent and the modelling would be correct. – Camilo Martin Feb 14 '13 at 07:12

3 Answers3

6

For documentation, I would just use the built-in documentation tags:

/// <summary>Exiting drag mode on mouse up</summary>
protected override void OnMouseUp(MouseEventArgs e)
{
    ...

For clarifying the intention I would just put a comment like

protected override void OnMouseUp(MouseEventArgs e)
{
    // not calling the base implementation
    ...
}

The line

// base.OnMouseUp(e);

makes an impression that the call is commented out temporarily (and perhaps someone forgot to restore it back)

Vlad
  • 35,022
  • 6
  • 77
  • 199
  • Yes, I agree that commenting out the code is bad, that's why I wanted something better. But, shouldn't the XML documentation state that it was inherited to cancel or alter its behavior? – Camilo Martin Mar 28 '12 at 21:04
  • 1
    @Camilo: well, it depends. If you think that it's good/interesting/important for the _users_ of your class to know this detail, you put it into the documentation. If this information is interesting only for _developers_ of your class, leave it as a comment inside the method. – Vlad Mar 28 '12 at 21:30
4

A comment like

// This method is intentionally blank because 
// we do not want the base class functionality

is much better than 

// base.SomeMethod();

The first comment clearly states why you did what you did, and the next developer who comes along won't have to wonder if the call to the base method was commented out accidentally.

If you have control over the base class, it may be better to remove that method and make the class more abstract. Then you can choose to only implement that functionality in child classes where it's needed.

Duane Theriot
  • 2,135
  • 1
  • 13
  • 16
  • I'm planning to do that on the method body. But what about the XML documentation? – Camilo Martin Mar 28 '12 at 21:11
  • 1
    It's up to you if you want to restate that in the XML documentation. You can explain again why you're overriding the base class if you think it's necessary. Generally, I would think a method named "OnMouseUp" is pretty self-explanatory and wouldn't require much header documentation. This isn't written in stone, but for me the XML comments usually tell "what" I'm doing, while comments within the method tell "why" I'm doing what I'm doing. – Duane Theriot Mar 28 '12 at 21:19
  • Thanks, that's reassuring. For being the first to comment on what to do about the XML comments, your answer is accepted. – Camilo Martin Mar 28 '12 at 21:28
1

Commenting out the base class call does, in my opinion the exact opposite of making intent clear. People will wonder why the commented line is still there, and whether it might still be of some use because you didn't delete it. So i would remove the commented out line.

You could document the override just like any other method and in the documentation, specify why exactly you left the method empty. You could write the reason into the method body as comment as well, i guess that's a matter of preference.

I think it depends on whether this information is only important for the developer maintaining the code or also for the user of the code (e.g. users of your library). In the case of an event that usually gets called by the operating system only (like in your example), putting it in the summary tag wouldn't really be necessary.

Still, if you need to override methods to disable behavior of the base class, maybe you should reconsider that part of your design. That behavior seems a bit unintuitive to me.

Botz3000
  • 39,020
  • 8
  • 103
  • 127
  • I agree that it does look ugly to have a commented-out line of code. I only did so because it was personal code and I didn't bother, but now I'd like any other developer to know why it's overriden. Do you suggest that I state why it was overriden in the `` tag? – Camilo Martin Mar 28 '12 at 21:07
  • I just thought about it again and i think it depends on the method. See my updated answer. – Botz3000 Mar 28 '12 at 21:27
  • I don't know if it's just me but I've never had to override my own classes' methods, and considered it to be only useful for users of code that disagree with the original developers (overriding your own stuff means something's wrong with the design in my opinion). I've just accepted another answer, but +1 because your edit raises an important point to consider. – Camilo Martin Mar 28 '12 at 21:35