Talk:Empty method over-rides
(Added my opinion on NOP over-rides.) |
|||
Line 6: | Line 6: | ||
:Ah, that's what I was looking for! How did I miss that? --[[User:Aidan|Aidan]] 09:44, 1 August 2009 (UTC) | :Ah, that's what I was looking for! How did I miss that? --[[User:Aidan|Aidan]] 09:44, 1 August 2009 (UTC) | ||
+ | |||
+ | I have a couple of points to contribute to this discussion | ||
+ | * Firstly, throwing exceptions is not okay. This is not a solution. Exceptions should happen '''only''' in exceptional circumstances. [http://userweb.cs.utexas.edu/users/EWD/ewd02xx/EWD215.PDF| GoTo statement considered harmful]. The one situation in which I'd argue this is acceptable is a NotImplementedException, which should never be handled, and indicates that the codebase isn't ready for the given operation, as well as giving an indication of what classes/methods need to be implemented for the desired functionality to work. | ||
+ | * The main issue with NOP over-rides is that they may violate the contract of the superclass, or require weakening the contract to the point where it guarantees practically nothing (it could still guarantee that no state is affected). Whether this is appropriate depends on the context. In the bark() method in the example, not barking when asked to bark() violates the implicit contract in the method name. On the other hand, a similar method, dealWithMailman() might have a NOP implementation/over-ride for WellBehavedDog. So I would argue that this is a [[Refused bequest smell]], and the comments on that page apply. | ||
+ | * Related to the above, it depends on whether it is an "action method", or a "signal method" (if there are proper terms for these, please let me know. Chances are "signal method" is actually called "bad OO method"). An "action method" being a method that performs an action. For instance, Stack::Push. There is a certain expectation that the item will be pushed onto the stack in this case. A "signal method" on the other hand is a method which is called to let a class do any handling it may need to of the given event. The canonical example of this is the [[Observer]] pattern's signal() method. The caller has no expectations of a specific result from calling the method, but calls it anyway to let other interested classes take action based on what just happened. I would argue that NOP action methods are bad, NOP signal methods are standard. --[[User:Lukas Korsika|Lukas Korsika]] 02:06, 16 July 2010 (UTC) |
Revision as of 02:06, 16 July 2010
I was just looking for people's opinions of this. Obviously empty methods are in general an indication of bad design, but they are tempting. What can be done instead? Are there situations where empty methods are okay? Would it be appropriate if the empty method threw an exception? This would still probably cause us to discard polymorphism, because we would have to check the instance type. What solutions are there that would allow us to maintain polymorphism? What principles or maxims are relevant here? --Aidan Bebbington 04:15, 1 August 2009 (UTC)
One discussion [1] resulted in a solution where the empty method throws an exception. This seems like complete rubbish to me because then clients of that class must use the instanceof operator or similar, and so polymorphism is lost. --Aidan 06:22, 1 August 2009 (UTC)
I would probably argue that empty methods are bad because they indicate that there might be a problem with your inheritance hierarchy. If you are using an empty method you should probably ask yourself why you don't want to implement it and if you should be using inheritance the way you are. I don't really like the idea of throwing an exception in an empty method. Kind of seems like the easy way out. As far as design principles go, Avoid no-op overrides probably applies here as well as Refused bequest smell. --Janina Voigt
- Ah, that's what I was looking for! How did I miss that? --Aidan 09:44, 1 August 2009 (UTC)
I have a couple of points to contribute to this discussion
- Firstly, throwing exceptions is not okay. This is not a solution. Exceptions should happen only in exceptional circumstances. GoTo statement considered harmful. The one situation in which I'd argue this is acceptable is a NotImplementedException, which should never be handled, and indicates that the codebase isn't ready for the given operation, as well as giving an indication of what classes/methods need to be implemented for the desired functionality to work.
- The main issue with NOP over-rides is that they may violate the contract of the superclass, or require weakening the contract to the point where it guarantees practically nothing (it could still guarantee that no state is affected). Whether this is appropriate depends on the context. In the bark() method in the example, not barking when asked to bark() violates the implicit contract in the method name. On the other hand, a similar method, dealWithMailman() might have a NOP implementation/over-ride for WellBehavedDog. So I would argue that this is a Refused bequest smell, and the comments on that page apply.
- Related to the above, it depends on whether it is an "action method", or a "signal method" (if there are proper terms for these, please let me know. Chances are "signal method" is actually called "bad OO method"). An "action method" being a method that performs an action. For instance, Stack::Push. There is a certain expectation that the item will be pushed onto the stack in this case. A "signal method" on the other hand is a method which is called to let a class do any handling it may need to of the given event. The canonical example of this is the Observer pattern's signal() method. The caller has no expectations of a specific result from calling the method, but calls it anyway to let other interested classes take action based on what just happened. I would argue that NOP action methods are bad, NOP signal methods are standard. --Lukas Korsika 02:06, 16 July 2010 (UTC)