Best practice for implementing a derived method that shouldn't be called [closed]

Question

I have a JAVA class A which has a method foo

abstract class A {
  abstract void foo();
}

I also have a derived class of A - MutableA. MutableA is a singleton object indicating that no update is necessary which is useful for reusing code flows. foo() should never be called on MutableA. What is the best way to achieve that:

1. Throw Unsupported exception
2. Do nothing (empty implementation)
3. This is a bad design what so ever.

Can someone recommend me what is the best practice in this case?

Melatonin

This really depends on the specifics of what foo is supposed to do. Any of those options might apply. – Ithaca

You can consider doing what the Guava Immutable* classes do, which is to mark such methods @Deprecated. This doesn't stop you calling them, but at least your IDE (if you use one) can give you a hint that something is up, provided that you have a reference to MutableA specifically. – Burletta

If you wanted to draw inspiration from Google's Guava, they throw UnsupportedOperationException if you call a mutating collection method on an immutable structure. They also mark the method as @Deprecated. – Limpet

Best practice for implementing a derived method that shouldn't be called ... is an oxymoron. There is no "best practice" for what is essentially a hack and violation of good inheritance design. Either implement inheritance correctly, or favour composition instead. Perhaps your design is too coarse grained. Of course, you can ignore what I've just said if you're stuck with some horrible legacy codebase/api where all sensibilities are thrown out the window – Amorino

Answer 1

Since you say "no update is necessary", it seems like an empty implementation is the way to go. What If I do:

A someA = createA();
someA.foo();           // make sure it's "foo"ed

I wouldn't know if I got back a MutableA or not, so I call foo just to be sure.

If it's forbidden to call foo on a MutableA I would go with UnsupportedOperationException or rethink the design. Perhaps you could wrap A in an AHandler that has knowledge of when and how to call foo on the instance that it wrapps.

Pneuma

Answer 2

I would consider rethinking the design.

Having an abstract method in the super class implies that subclasses should implement that method. Unless you plan on having a larger hierarchy in which case you should consider moving the foo method lower down in the hierarchy closer to the implementation.

If you are intent on keeping the foo methods location I would make MutableA abstract as well.

Phosphorous

I agree that the hierarchy should be changed -- perhaps an intermediate level should be introduced. If a method is available on an object then I expect I can use it as well. – Ardeth

It all depends because the use case may allow to call such method with empty implementation. Since we do not know the details the unswer cannot be precise – Close

Answer 3

You should consider havig a look at the Liskov substitution principle, which is one of the fundamental SOLID principles of good design.

The principle states that derived objects should not behave different than their parents. A common example for a violation of the Liskov principle is the derivation of a circle from an ellipse. Which is mathematically totally sensible and sound is considered to be bad design, for a circle derived from a ellipse would (for example) still expose the methods to set the width and the height, but it would behave oddly in some contexts. Consider the following

Something FooBar (Ellipse e)
{ 
    // do something with e
}

If we did not know that a caller passed a Circle rather than an Ellipse and we set both the width and the height our results may differ from what we expect, since the Circle either ignored SetHeight or it sets the width bot on SetWidth and SetHeight. Either way this behavior is unexpected if we expected to operate on an Ellipse. Hence the Liskov substitution principle.

Theron

Answer 4

It depends on which use the method is meant to. All alternatives you listed are virtually good.

By the way if you throws a UnsupportedOperation exception you're telling that this method is not supposed to be used. Using an exception is always a "message" of a deviation from the "normal", so you're defining a something-to-do with the base implementation of A.foo() but you're also breaking up this implementation with the subclass.

If you use an empty implementation you're making the subclass more usable inside an hypotetical flow, without breaking up with the past (the super class A), so you could being not aware about using subclass in every context you want.

In final analysis you said that MutableA is singleton, so I think you'll use it in a specific context, with a specific eye on it: so I would adopt the "exception" solution.

Tetryl

Answer 5

I'd use something like this:

public abstract class ReadableA {
}

public abstract class A extends ReadableA{
  abstract void foo();
}

public class MutableA extends ReadableA {
}

Everywhere in your code where you want to receive A, use ReadableA instead, unless you specifically want to call foo(). Then put A in your method signature. If you want to receive a MutableA, write a MutableA signature.

Examples:

public class UsageOfA {
  public void bar(ReadableA a) {
    // Can use both A and MutableA but a.foo() cannot be invoked.
  }
  public void bar(A a) {
    a.foo();
  }
  public void bar(MutableA a) {
    // a.foo() cannot be invoked.
  }
}

Mllly

Answer 6

I think throwing UnsupportedOperationException is the right choice here. Assume you 3 classes extending A .

class X extends A{
//foo allowed here
}
class Y extends A{
//foo allowed here
}
class Z extends A{
//foo allowed here
}
class MutableA extends A{
//foo NOT allowed here
}

Now from a design perspective, X,Y,Z and MutableA should behave in a consistent way with respect to foo(). I would discourage bringing in another class hierarchy just to make MutableA foofree. A simple soultion would be throw an UnsupportedOperationExceptionand let the caller know that the method cannot be supported by the calling Object. An empty implementation on the other hand is still valid and frankly doesn't fit in well from a design perspective because it is still a valid call from the calling object.

PS : I would also consider rethinking the design. Any method which is available but not supposed to be used is not part of a good design.

Dramaturge