First of all: You're right when questioning the use of instanceof
. It may have some use-cases, but whenever you feel tempted to use instanceof
, you should take a step back and check whether your design is really sound.
My suggestions are roughly in line with what Mena said in his answer. But from a conceptual view, I think that the direction of the dependency is a bit odd when you have to write a line like
instrument.play(musician);
instead of
musician.play(instrument);
A phrase like "an instrument can be played" IMHO suggests that the instruments are a parameter of a method, and not the object that the method is called on. They are "passive", in that sense. (One of your comments was also related to this, when you said that "the Instrument-class has an import on Musician", which doesn't seem right). But whether or not this is appropriate also depends on the real use-case. The example is very artificial and suggestive, and this may lead to suggestions for solutions that don't fit in the real world. The possible solutions for modeling this largely vary in the responsiblities, the question "Who knows what?", and how the modeled structures are intended to be used, and it's hard to give a general answer here.
However, considering that instruments can be played, it seems obvious that one could intruduce a simple method bePlayed()
in the Instrument
interface. This was already suggested in the other answers, leading to an implementation of the Musician
interface that simply plays the instrument:
public class GiftedMusician implements Musician
{
@Override
public void play(Instrument instrument)
{
instrument.bePlayed();
}
}
One of the open issues is:
Who (and how) decides whether a musician can play the instrument?
One pragmatic solution would be to let the musician know the instrument classes that he can play:
public class GiftedMusician implements Musician
{
private final Set<Class<?>> instrumentClasses =
new LinkedHashSet<Class<?>>();
<T extends Instrument> void learn(Class<T> instrumentClass)
{
instrumentClasses.add(instrumentClass);
}
void drinkLotsOfBeer()
{
instrumentClasses.clear();
}
@Override
public void play(Instrument instrument)
{
if (instrumentClasses.contains(instrument.getClass())
{
instrument.bePlayed();
}
else
{
System.out.println("I can't play the " + instrument.getClass());
}
}
}
In your EDIT, you opened a new degree of freedom for the design space: You mentioned that the instruments can be played in different ways (like the contrabass, with bow or fingers). This suggests that it may be appropriate to introduce a PlayingTechnique
class, as Mena also said in the comments.
The first shot could look like this
interface PlayingTechnique {
void applyTo(Instrument instrument);
}
But this raises two questions:
1. Which methods does the Instrument
interface offer?
This question could be phrased in more natural language: What do Instruments have in common?. Intuitively, one would say: Not much. They can be played, as already shown in the bePlayed()
method mentioned above. But this does not cover the different techniques, and these techniques may be highly specific for the particular class. However, you already mentioned some methods that the concrete classes could have:
Guitar#strumWithPick()
Bass#pluckString()
Piano#pressKey();
Contrabass#playWithBow();
Contrabass#pluckWithFingers()
So regarding the PlayingTechnique
class, one could consider adding some generics:
interface PlayingTechnique<T extends Instrument>
{
Class<?> getInstrumentClass();
void applyTo(T instrument);
}
and have different implementations of these:
class ContrabassBowPlayingTechnique
implements PlayingTechnique<Contrabass> {
@Override
public Class<?> getInstrumentClass()
{
return Contrabass.class;
}
@Override
public void applyTo(Contrabass instrument)
{
instrument.playWithBow();
}
}
class ContrabassFingersPlayingTechnique
implements PlayingTechnique<Contrabass> {
@Override
public Class<?> getInstrumentClass()
{
return Contrabass.class;
}
@Override
public void applyTo(Contrabass instrument)
{
instrument.pluckWithFingers();
}
}
(Side note: One could consider generalizing this even further. This would roughly mean that the Instrument
interface would have several sub-interfaces, like StringInstrument
and KeyInstrument
and WindInstrument
, each offering an appropriate set of more specific methods, like
StringInstrument#playWithBow()
StringInstrument#playWithFingers()
While technically possible, this would raise questions like whether a Guitar may be played with the bow, or a Violin may be played with the fingers - but this goes beyond what can seriously be considered based on the artificial example)
The GiftedMusician
class could be adjusted accordingly:
public class GiftedMusician implements Musician
{
private final Set<PlayingTechnique<?>> playingTechniques =
new LinkedHashSet<PlayingTechnique<?>>();
<T extends Instrument> void learn(PlayingTechnique<T> playingTechnique)
{
playingTechniques.add(playingTechnique);
}
void drinkLotsOfBeer()
{
playingTechniques.clear();
}
@Override
public void play(Instrument instrument)
{
for (PlayingTechnique<?> playingTechnique : playingTechniques)
{
if (playingTechnique.getInstrumentClass() == instrument.getClass())
{
// May need to cast here (but it's safe)
playingTechnique.applyTo(instrument);
return;
}
}
System.out.println("I can't play the " + instrument.getClass());
}
}
Still, there is a second open question:
2. Who decides (when and how) which PlayingTechique
is applied?
In the current form, a gifted musician could learn two playing techniques for the same instrument class:
giftedMusician.learn(new ContrabassBowPlayingTechnique());
giftedMusician.learn(new ContrabassFingersPlayingTechnique());
// Which technique will he apply?
giftedMusician.play(contrabass);
But whether the decision is made by the Musician
(maybe based on some "proficiency" that is associated with each technique), or from the outside, will depend on the real-world problem that you are actually trying to solve.