views:

276

answers:

6

I am dealing with a large codebase that has a lot of classes and a lot of abstract methods on these classes. I am interested in peoples opinions about what I should do in the following situation.

If I have a class Parent-A with an abstract method. There will only be 2 children. If Child-B implements AbstractMethodA but Child-B does not as it doesnt apply.

Should I

  1. Remove the abstract keyword from parent and use virtual or dynamic?
  2. Provide a empty implementation of the method.
  3. Provide an implementation that raises an error if called.
  4. Ignore the warning.

Edit: Thanks for all the answers. It confirmed my suspicion that this shouldn't happen. After further investigation it turns out the methods weren't used at all so I have removed them entirely.

A: 

make it virtual empty in base class and override it in children.

Mladen Prajdic
+9  A: 

If AbstractMethodA does not apply to Child-B, then Child-B should not be inheriting from Parent-A.

Or to take the contrapositive, if Child-B inherits from Parent-A, and AbstractMethodA does not apply to the child, then it should not be in the parent either.

By putting a method in Parent-A, you are saying that the method applies to Parent-A and all its children. That's what inheritance means, and if you use it to mean something different, you will end up in a serious dispute with your compiler.

[Edit - that said, Mladen Prajdic's answer is fine if the method does apply, but should do nothing for one or more of the classes involved. A method which does nothing is IMO not the same thing as a method which is not applicable, but maybe we don't mean the same thing by "doesn't apply"]

Another technique is to implement the method in Child-B anyway, but have it do something drastic like always returning failure, or throw an exception, or something. It works, but should be regarded as a bit of a bodge rather than a clean design, since it means that callers need to know that the thing they have that they're treating as Parent-A is really a child-B and hence they shouldn't call AbstractMethodA. Basically you've discarded polymorphism, which is the main benefit of OO inheritance. Personally I prefer doing it this way over having an exception-throwing implementation in the base class, because then a child class can't "accidentally" behave badly by "forgetting" to implement the method at all. It has to implement it, and if it implements it to not work then it does so explicitly. A bad situation should be noisy.

Steve Jessop
A: 

You could use interfaces. Then Child-A and Child-B can both implement different methods and still inherit from Parent-A. Interfaces work like abstract methods in that they force the class to implement them.

Lars Truijens
+2  A: 

If implementation in descendants is not mandatory then you should go for 1+2 (i.e. empty virtual method in ancestor)

Oliver Giesen
A: 

If some subclasses (B1, B2, ...) of A are used for a different subset of its methods than others (C1, C2, ...), one might say that A can be split in B and C.

I don't know Delphi too well (not at all :) ), but I thought that just like e.g. in Java and COM, a class can 'implement' multiple interfaces. In C++ this can only be achieved by multiply inheriting abstract classes.

More concrete: I would create two abstract classes (with abstract methods), and change the inheritance tree.

If that's not possible, a workaround could be an "Adapter": an intermediate class A_nonB_ with all B methods implemented empty (and yielding a warning on calling them), and A_nonC_. Then change the inheritance tree to solve your problem: B1, B2, ... inherit from A_nonC_ and C1, C2,... inherit from A_NonB_.

xtofl
+1  A: 

I think that, generally speaking, you shouldn't inherit from the abstract class if you are unable to implement all of the abstract methods in the first place, but I understand that there are some situations where it still makes senseto do that, (see the Stream class and its implementations).

I think you should just create implementations of these abstract methods that throw a NotImplementedException.

You can also try using ObsoleteAttribute so that calling that particular method would be a compile time error (on top of throwing NotImplementedException of course). Note that ObsoleteAttribute is not quite meant to be used for this, but I guess if you use a meaningful error message with comments, it's alright.

Obligatory code example:

[Obsolete("This class does not implement this method", true)]
public override string MyReallyImportantMethod()
{
    throw new NotImplementedException("This class does not implement this method.");
}
DrJokepu