8

As you know the 'program to an interface' design principle broadly prefers supertypes instead of concrete types or implementations.

Is it consistent with the principle to use instanceof in a Java program to derive a concrete type from a supertype?

In my application, Storehouse is an abstract supertype class with a couple of private variables and public getters and setters.

ConcreteStorehouseA inherits from Storehouse and has a lot of concrete methods and variables. ConcreteStorehouseB is similar but different.

My application receives a Storehouse. However, Storehouse is not a useful type to operate on. Because the only really useful methods are contained in the concrete types, I use instanceof as follows:

if (storehouse instanceof ConcreteStorehouseA) {
    ConcreteStorehouseA concreteStorehouseA = (ConcreteStorehouseA) storehouse;
    // perform operations on the concrete type's useful methods and variables

Is using instanceof compatible with the principle?

Edit:

In essence the application is a dice simulator for a table top RPG, Shadowrun. The concrete types are the different test types - Success Test, Opposed Test, Extended Test - which all have very different factors and parameters for their successful operation. The supertype essentially contains the dice pool!

Arvanem
  • 1,043
  • 12
  • 22
  • No, I don't think it is consistent with the principle. It is sometimes necessary, but it will be hard for you to "code to the interface" with your current design since your child classes differ so. So my opinion is you either change your design or you drop the code to the interface goal. 1+ Kudos though for trying to achieve this noble goal. – Hovercraft Full Of Eels Feb 11 '11 at 21:23
  • You may want to use delegation instead of subtyping, here. If the different test types have nothing in common but that they are using a dice pool, they should delegate to (asking) this (common?) dice pool instead of each being a (separate?) dice pool. – Paŭlo Ebermann Feb 11 '11 at 22:22

7 Answers7

10

As a rule of thumb, that "program to interfaces" principle that you mentioned can be translated into: import only the interface type, don't have any compile-time dependency on subclasses.

Therefore, the answer to your question would be definitely no. You are not programming to interfaces since you cast to concrete types.

Costi Ciudatu
  • 37,042
  • 7
  • 56
  • 92
8

You've said it yourself:

My application receives a Storehouse. However, Storehouse is not a useful type to operate on. Because the only really useful methods are contained in the concrete types

In other words, your Storehouse abstraction isn't buying you anything... why do you have it?

Could you create abstract methods in Storehouse, implemented in each concrete class, which would then let you treat the concrete types the same way in your client code? That's the goal of the abstraction.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Thanks Jon. Yes, the abstract methods proposal you suggest is viable. I think half the abstract methods would be blank implementations in ConcreteStorehouseA and the other half would be blank implementations in ConcreteStorehouseB. Is that still a useful abstraction I wonder? – Arvanem Feb 11 '11 at 21:31
  • 1
    @Arvanem: Creating methods in the base class that only make sense for one of the concrete classes is *probably* an equally bad design... – Oliver Charlesworth Feb 11 '11 at 21:34
  • @Arvanem: I wouldn't think that it's as useful an abstraction at that point, no. If a lot of what it defines is undefined in any given implementation then it doesn't really accurately describe the "type" of the implementation. It's providing common functionality to its sub-types to maintain DRY well enough, but it's not really a definition of a `Storehouse` if it's not used by storehouses. – David Feb 11 '11 at 21:35
  • @Arvanem: It sounds like you simply can't treat the two subclasses in a common way - so don't try to. Have different methods dealing with each type. – Jon Skeet Feb 11 '11 at 21:36
  • @Oli: Thanks. Well, time to reconsider. I'll edit the question to clarify what Storehouse represents for those curious to know. – Arvanem Feb 11 '11 at 21:36
2

Not really. If your method has wildly different behaviour depending on the type that it receives, then polymorphism is buying you nothing. You should consider two separate overloaded methods, one taking a ConcreteStorehouseA as an argument, and the other taking a ConcreteStorehouseB.

Oliver Charlesworth
  • 267,707
  • 33
  • 569
  • 680
2

It is not always a sin. Suppose you are implementing a spec, which says, if x is A do this, else if x is Y do that. It's better to have your code look like the spec. To artificially cut it into little pieces and places them into difference sources, is not only pretentious, but also complicated, unsafe, hard to understand and hard to maintain.

Programming concerns are multi-dimensional. Programming languages are one dimensional, at least for now. It is an art how to organize related concerns closely. Do not buy into the dogma that concerns must be decomposed according to classes, then a concern can at most reference one class, and it must reside inside that class.

Last century there was this great taboo in software engineering, according to the experts who knew how to write better softwares: that is, code change should be avoided at all cost. If you are going to change some source code you wrote previously, the universe could collapse into a peanut at any moment. So you better design a perfect architecture from the start, and then any change in requirement can be done by adding a new/clean class, without touching existing code base.

That was, let me be very precise in wording here, total nonsense, even back then.

Today, we have much better tools, and code change is not only safe, it's even encouraged. The best way to make sure code change is easy tomorrow due to unforeseeable reasons, is to keep code today as simple as possible. Write code in a way that you can understand even in a comma.

irreputable
  • 44,725
  • 9
  • 65
  • 93
1

It can be difficult to be sure without insight into what operations you are performing, but a more elegant design would be to have StoreHouse define method signatures for the different "operations". Then, instead of the if(instanceof) checks, you just execute the operations, and the concreteStorehouses would implement those operations.

public abstract class Storehouse
//add these definitions;
public void operation1();
public void operation2();

public class ConcreteStorehouseA

public void operation1() {
   //do operation
}

public void operation2() {
   //do operation
}

public class ConcreteStorehouseB

public void operation1() {
   //do operation
}

public void operation2() {
   //do operation
}

In your calling code, instead of:

if (storehouse instanceof ConcreteStorehouseA) {
    ConcreteStorehouseA concreteStorehouseA = (ConcreteStorehouseA) storehouse;
    // perform operations
} else if (storehouse instanceof ConcreteStorehouseB) {
    ConcreteStorehouseB concreteStorehouseB = (ConcreteStorehouseB) storehouse;
    // perform operations
}

You can then use polymorphism to do the operations without caring which implementation is being executed.

storehouse.operation1();
storehouse.operation2();

where concreteStorehouseA and ConcreteStorehouseB have defined implementations for those operations.

philippe
  • 546
  • 1
  • 3
  • 3
1

It sounds like the Storehouse abstraction isn't really a proper abstraction at all in this case. As a type by itself it isn't really providing you with anything.

One thing that may help is to try not to think of your implementations as "types" at all. The "type" is the abstraction that the class is implementing. (Basically, pull apart the terms "class" and "type" in your thinking.) So the type you're working with is a Storehouse. ConcreteStorehouseA is a Storehouse, and so is ConcreteStorehouseB. So the question becomes, what is a Storehouse? That's the type that defines what each one is.

In this particular case it sounds like the different Storehouse implementations are so different (at least in their current implementation) that they don't really share a worth-while abstraction. The abstract "type" in this case is just providing some common functionality by means of inheritance rather than actually abstracting the type.

To put it another way, it sounds like the Storehouse implementations here are a classic example of the Liskov Substitution motivational poster here: http://www.lostechies.com/blogs/derickbailey/archive/2009/02/11/solid-development-principles-in-motivational-pictures.aspx

David
  • 208,112
  • 36
  • 198
  • 279
  • +1 for your insight into the real use of the type that is happening - inheritance rather than abstracting the type. Thanks – Arvanem Feb 11 '11 at 21:42
  • 1
    @Arvanem: It's an endlessly interesting topic for me, really. I've had some excellent lunch conversations with colleagues about inheritance vs. composition, polymorphism through interfaces, etc. In this particular case it sounds like it might do well to use composition to achieve the common functionality and avoid the inheritance as it falsely presents an abstraction. If you can abstractly define a `Storehouse` interface then that would be great, but such a common abstraction isn't _always_ possible in every problem domain. – David Feb 11 '11 at 21:48
0

As various people have pointed out, this is not programming to the interface. Apparently, the interface abstraction is too weak for your requirements.

There are various ways to deal with this, among which:

  • If the interface is under your control, you can expand it. Consider whether that's worth it, it may be too high a price to pay, especially if the next bullet point holds.
  • If you end up using only one kind of concrete warehouse, you can just use that directly.
  • If you need to support various kinds of warehouses, a good alternative is to define your own interface that provides exactly the abstraction you need it and write a thin wrapper for each kind of concrete class (adapter pattern).
Barend
  • 17,296
  • 2
  • 61
  • 80