33

I have a base class, Base. It has two subclasses, Sub1 and Sub2. Each subclass has some additional methods. For example, Sub1 has Sandwich makeASandwich(Ingredients... ingredients), and Sub2 has boolean contactAliens(Frequency onFrequency).

Since these methods take different parameters and do entirely different things, they're completely incompatible, and I can't just use polymorphism to solve this problem.

Base provides most of the functionality, and I have a large collection of Base objects. However, all Base objects are either a Sub1 or a Sub2, and sometimes I need to know which they are.

It seems like a bad idea to do the following:

for (Base base : bases) {
    if (base instanceof Sub1) {
        ((Sub1) base).makeASandwich(getRandomIngredients());
        // ... etc.
    } else { // must be Sub2
        ((Sub2) base).contactAliens(getFrequency());
        // ... etc.
    }
}

So I came up with a strategy to avoid this without casting. Base now has these methods:

boolean isSub1();
Sub1 asSub1();
Sub2 asSub2();

And of course, Sub1 implements these methods as

boolean isSub1() { return true; }
Sub1 asSub1();   { return this; }
Sub2 asSub2();   { throw new IllegalStateException(); }

And Sub2 implements them in the opposite way.

Unfortunately, now Sub1 and Sub2 have these methods in their own API. So I can do this, for example, on Sub1.

/** no need to use this if object is known to be Sub1 */
@Deprecated
boolean isSub1() { return true; }

/** no need to use this if object is known to be Sub1 */
@Deprecated
Sub1 asSub1();   { return this; }

/** no need to use this if object is known to be Sub1 */
@Deprecated
Sub2 asSub2();   { throw new IllegalStateException(); }

This way, if the object is known to be only a Base, these methods are un-deprecated, and can be used to "cast" itself to a different type so I can invoke the subclass's methods on it. This seems elegant to me in a way, but on the other hand, I'm kind of abusing Deprecated annotations as a way to "remove" methods from a class.

Since a Sub1 instance really is a Base, it does make sense to use inheritance rather than encapsulation. Is what I'm doing good? Is there a better way to solve this problem?

  • 12
    All 3 classes now have to know about each other. Adding Sub3 would involve a lot of code changes, and adding Sub10 would be downright painful – Dan Pichelman Apr 29 '15 at 19:21
  • 15
    It would help a lot if you gave us real code. There's situations where it's appropriate to take decisions based on the particular class of something, but it's impossible to tell if you're justified in what you're doing with such contrived examples. For whatever it's worth, what you want is a Visitor or tagged union. – Doval Apr 29 '15 at 21:07
  • 1
    Sorry, I thought it would make things easier to give simplified examples. Maybe I'll post new question with a broader scope of what I want to do. – codebreaker Apr 29 '15 at 21:13
  • 12
    You're just reimplementing casting and instanceof, in a way that requires lots of typing, is error-prone, and makes it hard to add more subclasses. – immibis Apr 30 '15 at 0:32
  • 5
    If Sub1 and Sub2 cannot be used interchangeably, then why do you treat them as such? Why not keep track of your 'sandwich-makers' and 'alien-contacters' separately? – Pieter Witvoet Apr 30 '15 at 8:10

10 Answers 10

27

It doesn't always make sense to add functions to the base class, as suggested in some of the other answers. Adding too many special case functions can result in binding otherwise unrelated components together.

For example I might have an Animal class, with Cat and Dog components. If I want to be able to print them, or show them in the GUI it might be overkill for me to add renderToGUI(...) and sendToPrinter(...) to the base class.

The approach you are using, using type-checks and casts, is fragile - but at least does keep the concerns separated.

However if you find yourself doing these types of checks/casts often then one option is to implement the visitor / double-dispatch pattern for it. It looks kind of like this:

public abstract class Base {
  ...
  abstract void visit( BaseVisitor visitor );
}

public class Sub1 extends Base {
  ...
  void visit(BaseVisitor visitor) { visitor.onSub1(this); }
}

public class Sub2 extends Base {
  ...
  void visit(BaseVisitor visitor) { visitor.onSub2(this); }
}

public interface BaseVisitor {
   void onSub1(Sub1 that);
   void onSub2(Sub2 that);
}

Now your code becomes

public class ActOnBase implements BaseVisitor {
    void onSub1(Sub1 that) {
       that.makeASandwich(getRandomIngredients())
    }

    void onSub2(Sub2 that) {
       that.contactAliens(getFrequency());
    }
}

BaseVisitor visitor = new ActOnBase();
for (Base base : bases) {
    base.visit(visitor);
}

The main benefit is that if you add a subclass, you'll get compile errors rather than silently missing cases. The new visitor class also becomes a nice target for pulling functions into. For example it might make sense to move getRandomIngredients() into ActOnBase.

You can also extract the looping logic: For example the above fragment might become

BaseVisitor.applyToArray(bases, new ActOnBase() );

A little further massaging and using Java 8's lambdas and streaming would let you get to

bases.stream()
     .forEach( BaseVisitor.forEach(
       Sub1 that -> that.makeASandwich(getRandomIngredients()),
       Sub2 that -> that.contactAliens(getFrequency())
     ));

Which IMO is pretty much as neat looking and succinct as you can get.

Here is a more complete Java 8 example:

public static abstract class Base {
    abstract void visit( BaseVisitor visitor );
}

public static class Sub1 extends Base {
    void visit(BaseVisitor visitor) { visitor.onSub1(this); }

    void makeASandwich() {
        System.out.println("making a sandwich");
    }
}

public static class Sub2 extends Base {
    void visit(BaseVisitor visitor) { visitor.onSub2(this); }

    void contactAliens() {
        System.out.println("contacting aliens");
    }
}

public interface BaseVisitor {
    void onSub1(Sub1 that);
    void onSub2(Sub2 that);

    static Consumer<Base> forEach(Consumer<Sub1> sub1, Consumer<Sub2> sub2) {

        return base -> {
            BaseVisitor baseVisitor = new BaseVisitor() {

                @Override
                public void onSub1(Sub1 that) {
                    sub1.accept(that);
                }

                @Override
                public void onSub2(Sub2 that) {
                    sub2.accept(that);
                }
            };
            base.visit(baseVisitor);
        };
    }
}

Collection<Base> bases = Arrays.asList(new Sub1(), new Sub2());

bases.stream()
     .forEach(BaseVisitor.forEach(
             Sub1::makeASandwich,
             Sub2::contactAliens));
  • +1: This kind of thing is commonly used in languages that have first-class support for sum-types and pattern matching, and it is a valid design in Java, even though it is syntactically very ugly. – Mankarse Apr 30 '15 at 8:49
  • @Mankarse A little syntactic sugar can make it very far from ugly - I've updated it with an example. – Michael Anderson Apr 30 '15 at 9:05
  • Let's say hypothetically the OP changes their mind and decides to add a Sub3. Doesn't visitor have the exact same problem as instanceof? Now you need to add onSub3 to the visitor and it breaks if you forget. – Radiodef Apr 30 '15 at 17:04
  • 1
    @Radiodef An advantage of using the visitor pattern over switching on type directly is that once you've added onSub3 to the visitor interface, you get a compile error in every location you create a new Visitor that hasn't yet been updated. In contrast switching on type will at best generate a run-time error - which can be trickier to trigger. – Michael Anderson May 1 '15 at 5:09
  • 1
    @FiveNine, if you're happy to add that complete example into the end of the answer that might help others. – Michael Anderson May 6 '15 at 1:46
83

From my perspective: your design is wrong.

Translated to natural language, you are saying the following:

Given we have animals, there are cats and fish. animals have properties, which are common to cats and fish. But that's not enough: there are some properties, which differentiate cat from fish, therefore you need to subclass.

Now you have the problem, that you forgot to model movement. Okay. That's relatively easy:

for(Animal a : animals){
   if (a instanceof Fish) swim();
   if (a instanceof Cat) walk();
}

But that is a wrong design. The correct way would be:

for(Animal a : animals){
    animal.move()
}

Where move would be shared behavior implemented differently by each animal.

Since these methods take different parameters and do entirely different things, they're completely incompatible, and I can't just use polymorphism to solve this problem.

This means: your design is broken.

My recommendation: refactor Base, Sub1 and Sub2.

  • 8
    If you would offer actual code, I could make recommendations. – Thomas Junk Apr 29 '15 at 19:41
  • 9
    @codebreaker If you agree that your example wasn't good, I recommend accepting this answer and then writing a new question. Don't revise the question to have a different example or the existing answers won't make sense. – Moby Disk Apr 29 '15 at 20:31
  • 16
    @codebreaker I think this "solves" the problem by answering the real question. The real question being: How do I solve my problem? Refactor your code properly. Refactor so that you .move() or .attack() and properly abstract that part - instead of having .swim(), .fly(), .slide(), .hop(), .jump(), .squirm(), .phone_home(), etc. How do you refactor properly? Unknown without better examples... but still generally the right answer - unless example code and more details suggest otherwise. – WernerCD Apr 29 '15 at 23:00
  • 11
    @codebreaker If your class hierarchy leads you to situations like this, then by definition it doesn't make sense – Patrick Collins Apr 30 '15 at 3:00
  • 7
    @codebreaker Related to Crisfole's last comment, there's another way of looking at it that might help. For example, with the move vs fly/run/etc, that can be explained in one sentence as: You should be telling the object what to do, not how to do it. In terms of accessors, as you mention is more like the real case in a comment here, you should ask the object questions, not inspect its state. – Izkata Apr 30 '15 at 18:51
9

It's a little hard to imagine a circumstance where you have a group of things and want them to either make a sandwich or contact aliens. In most cases where you find such casting you will operate with one type - e.g. in clang you filter a set of nodes for declarations where getAsFunction returns non-null, rather than doing something different for each node in the list.

It might be that you need a perform a sequence of action and it's not actually relevant that the objects performing the action are related.

So instead of a list of Base, work on the list of actions

for (RandomAction action : actions)
   action.act(context);

where

interface RandomAction {
    void act(Context context);
} 

interface Context {
    Ingredients getRandomIngredients();
    double getFrequency();
}

You can, if appropriate, have Base implement a method to return the action, or whatever other means you need to select the action from the instances in your base list (since you say you can't use polymorphism, so presumably the action to take is not a function of the class but some other property of the bases; otherwise you'd just give Base the act(Context) method )

  • 2
    You get that my sort of absurd methods are designed to show the differences in what the classes can do once their subclass is known (and that they have nothing to do with each other), but I think having a Context object just makes matters worse. I might as well just be passing in Objects to the methods, casting them, and hoping they're the right type. – codebreaker Apr 29 '15 at 20:38
  • 1
    @codebreaker you really need to clarify your question then - in most systems there would be a valid reason for calling a bunch of functions agnostic of what they do; for example to process rules on an event or perform a step in a simulation. Usually such systems have a context in which the actions occur. If the 'getRandomIngredients' and 'getFrequency' are not related, then then shouldn't be in the same object, and you need yet a different approach, such as having the actions capture the source of ingredients or frequency. – Pete Kirkham Apr 29 '15 at 22:18
  • 1
    You're right, and I don't think I can salvage this question. I ended up picking a bad example, so I'll probably just post another one. The getFrequency() and getIngredients() were just placeholders. I probably should have just put "..." as the arguments to make it more apparent that I wouldn't know what they are. – codebreaker Apr 29 '15 at 23:44
  • This is IMO the right answer. Understand that the Context doesn't need to be a new class, or even an interface. Usually the context is just the caller, so calls are in the form action.act(this). If getFrequency() and getRandomIngredients() are static methods, you might not even need a context. This is how I would implement say, a job queue, which your problem sounds an awful lot like. – QuestionC Apr 30 '15 at 21:58
  • Also this is mostly identical to the visitor pattern solution. The difference is you're implementing the Visitor::OnSub1()/Visitor::OnSub2() methods as Sub1::act()/Sub2::act() – QuestionC Apr 30 '15 at 22:01
4

How about if you have your subclasses implement one or more interfaces that define what they can do ? Something like this:

interface SandwichCook
{
    public void makeASandwich(String[] ingredients);
}

interface AlienRadioSignalAwarable
{
    public void contactAliens(int frequency);

}

Then your classes will look like this:

class Sub1 extends Base implements SandwichCook
{
    public void makeASandwich(String[] ingredients)
    {
        //some code here
    }
}

class Sub2 extends Base implements AlienRadioSignalAwarable
{
    public void contactAliens(int frequency)
    {
        //some code here
    }
}

And your for-loop will become:

for (Base base : bases) {
    if (base instanceof SandwichCook) {
        base.makeASandwich(getRandomIngredients());
    } else if (base instanceof AlienRadioSignalAwarable) {
        base.contactAliens(getFrequency());
    }
}

Two major advantages to this approach:

  • no casting involved
  • you can have each subclass implement as many interfaces as you want, which does provide some flexibility for future changes.

PS: Sorry for the names of the interfaces, I couldn't think of anything cooler at that particular moment :D .

  • 3
    This doesn't actually solve the problem. You still need the cast in the for loop. (If you don't then you don't need the cast in OP's example either). – Taemyr Apr 30 '15 at 14:03
2

The approach can be a good one in cases where almost any type within a family will either be directly usable as an implementation of some interface which meets some criterion or can be used to create an implementation of that interface. The built-in collection types would IMHO have benefited from this pattern, but since they don't for purposes of example I'll invent a collection interface BunchOfThings<T>.

Some implementations of BunchOfThings are mutable; some aren't. In many cases, an object Fred might want to hold something it can use as a BunchOfThings and know that nothing other than Fred will be able to modify it. This requirement may be satisfied in two ways:

  1. Fred knows that it holds the only references to that BunchOfThings, and no outside reference to that BunchOfThings ir its internals exists anywhere in the universe. If nobody else has a reference to the BunchOfThings or its internals, nobody else will be able to modify it, so the constraint will be satisfied.

  2. Neither the BunchOfThings, nor any of its internals to which outside references exist, can be modified via any means whatsoever. If absolutely nobody can modify a BunchOfThings, then the constraint will be satisfied.

One way of satisfying the constraint would be to unconditionally copy any object received (recursively processing any nested components). Another would be to test whether a received object promises immutability and, if not, make a copy of it, and do likewise with any nested components. An alternative, which is apt to be cleaner than the second and faster than the first, is to offer an AsImmutable method which asks an object to make an immutable copy of itself (using AsImmutable on any nested components that support it).

Related methods may also be provided for asDetached (for use when code receives an object and doesn't know if it will want to mutate it, in which case a mutable object should be replaced with a new mutable object, but an immutable object can be kept as-is), asMutable (in cases where an object knows that it will hold an object earlier returned from asDetached, i.e. either an unshared reference to a mutable object or a sharable reference to a mutable one), and asNewMutable (in cases where code receives an outside reference and knows it's going to want to mutate a copy of the data therein--if the incoming data is mutable there's no reason to start by making an immutable copy which is going to be immediately used to create a mutable copy and then abandoned).

Note that while the asXX methods may return slightly different types, their real role is to ensure that the returned objects will satisfy program needs.

0

Ignoring the issue of whether you have a good design or not, and supposing it's good or at least acceptable, I would want to consider the capabilities of the subclasses, not the type.

Therefore, either:


Move some knowledge of the existence of sandwiches and aliens into the base class, even though you know some instances of the base class can't do it. Implement it in the base class to throw exceptions, and change the code to:

if (base.canMakeASandwich()) {
    base.makeASandwich(getRandomIngredients());
    // ... etc.
} else { // can't make sandwiches, must be able to contact aliens
    base.contactAliens(getFrequency());
    // ... etc.
}

Then one or both subclasses overrides canMakeASandwich(), and only one implements each of makeASandwich() and contactAliens().


Use interfaces, not concrete subclasses, to detect what capabilities a type has. Leave the base class alone, and change the code to:

if (base instanceof SandwichMaker) {
    ((SandwichMaker)base).makeASandwich(getRandomIngredients());
    // ... etc.
} else { // can't make sandwiches, must be able to contact aliens
    ((AlienContacter)base).contactAliens(getFrequency());
    // ... etc.
}

or possibly (and feel free to ignore this option if it doesn't suit your style, or for that matter any Java style you'd consider sensible):

try {
    ((SandwichMaker)base).makeASandwich(getRandomIngredients());
} catch (ClassCastException e) {
    ((AlienContacter)base).contactAliens(getFrequency());
}

Personally I don't usually like the latter style of catching a half-expected exception, because of the risk of inappropriately catching a ClassCastException coming out of getRandomIngredients or makeASandwich, but YMMV.

  • 2
    Catching ClassCastException is just ew. That's the entire purpose of instanceof. – Radiodef Apr 30 '15 at 17:22
  • @Radiodef: true, but one purpose of < is to avoid catching ArrayIndexOutOfBoundsException, and some folks decide to do that too. No accounting for taste ;-) Basically my "problem" here is I work in Python, where the preferred style usually is that catching any exception is preferable to testing in advance whether the exception will occur, no matter how simple the advance test would be. Maybe the possibility just isn't worth mentioning in Java. – Steve Jessop Apr 30 '15 at 17:23
  • @SteveJessop »It's easier to ask for forgiveness than for permission« is the slogan ;) – Thomas Junk Apr 30 '15 at 23:31
0

Here we have an interesting case of a base class that downcasts itself to its own derived classes. We know full well this is normally bad, but if we want to say we found a good reason, let us look and see what the constraints on this might be:

  1. We can cover all the cases in the base class.
  2. No foreign derived class would have to add a new case, ever.
  3. We can live with the policy for which call to make being under the control of the base class.
  4. But we know from our science that the policy of what to do in a derived class for a method not in the base class is that of the derived class and not the base class.

If 4 then we have: 5. The policy of the derived class is always under the same political control as the policy of the base class.

2 and 5 both directly imply that we can enumerate all derived classes, which means that there aren't supposed to be any outside derived classes.

But here's the thing. If they're all yours you can replace the if with an abstraction that is a virtual method call (even if it's a nonsense one) and get rid of the if and self-cast. Therefore, don't do it. A better design is available.

-1

Put an abstract method in the base class which does one thing in Sub1 and the other thing in Sub2, and call that abstract method in your mixed loops.

class Sub1 : Base {
    @Override void doAThing() {
        this.makeASandwich(getRandomIngredients());
    }
}

class Sub2 : Base {
    @Override void doAThing() {
        this.contactAliens(getFrequency());
    }
}

for (Base base : bases) {
    base.doAThing();
}

Note that this may require other changes depending on how getRandomIngredients and getFrequency are defined. But, honestly, inside the class that contacts aliens is probably a better place to define getFrequency anyway.

Incidentally, your asSub1 and asSub2 methods are definitely bad practice. If you're going to do this, do it by casting. There's nothing these methods give you that casting doesn't.

  • 2
    Your answer suggests you didn't read through the question completely: polymorphism just won't cut it here. There are several methods on each of Sub1 and Sub2, these are just examples, and "doAThing" doesn't really mean anything. It doesn't correspond to anything that I'd be doing. Additionally, as for your last sentence: how about guaranteed type safety? – codebreaker Apr 29 '15 at 23:35
  • @codebreaker - It corresponds exactly to what you're doing in the loop in the example. If that has no meaning, don't write the loop. If it doesn't make sense as a method it doesn't make sense as a loop body. – Random832 Apr 29 '15 at 23:38
  • 1
    And your methods "guarantee" type safety the same way casting does: by throwing an exception if the object is the wrong type. – Random832 Apr 29 '15 at 23:40
  • I realize I picked a bad example. The problem is most of the methods in the subclasses are more like getters than mutative methods. These classes don't do things themselves, just provide data, mostly. Polymorphism won't help me there. I'm dealing with producers, not consumers. Also: throwing an exception is a runtime guarantee, which isn't as good as compile-time. – codebreaker Apr 29 '15 at 23:41
  • 1
    My point is your code only provides a runtime guarantee as well. There's nothing stopping someone from failing to check isSub2 before calling asSub2. – Random832 Apr 29 '15 at 23:43
-2

You could push both makeASandwich and contactAliens into the base class and then implement them with dummy implementations. Since the return types/arguments can't be resolved to a common base class.

class Sub1 extends Base{
    Sandwich makeASandwich(Ingredients i){
        //Normal implementation
    }
    boolean contactAliens(Frequency onFrequency){
        return false;
    }
 }
class Sub2 extends Base{
    Sandwich makeASandwich(Ingredients i){
        return null;
    }
    boolean contactAliens(Frequency onFrequency){
       // normal implementation
    }
 }

There are obvious downsides to this sort of thing, like what that means for the contract of the method and what you can and cannot infer about the context of the result. You can't think since I failed to make a sandwich the ingredients got used up in the attempt etc.

  • 1
    I did think about doing this, but it violates the Liskov Substitution Principle, and isn't as nice to work with, e.g., when you type "sub1." and the autocomplete comes up, you see makeASandwich but not contactAliens. – codebreaker Apr 29 '15 at 20:16
-5

What you are doing is perfectly legitimate. Do not pay attention to the naysayers who merely reiterate dogma because they read it in some books. Dogma has no place in engineering.

I have employed the same mechanism a couple of times, and I can say with confidence that the java runtime could have also done the same thing in at least one place that I can think of, thus improving performance, usability, and readability of code that uses it.

Take for example java.lang.reflect.Member, which is the base of java.lang.reflect.Field and java.lang.reflect.Method. (The actual hierarchy is a bit more complicated than that, but that's irrelevant.) Fields and methods are vastly different animals: one has a value that you can get or set, while the other has no such thing, but it can be invoked with a number of parameters and it may return a value. So, fields and methods are both members, but the things you can do with them are about as different from each other as making sandwiches vs. contacting aliens.

Now, when writing code that uses reflection we very often have a Member in our hands, and we know that it is either a Method or a Field, (or, rarely, something else,) and yet we have to do all the tedious instanceof to figure out precisely what it is and then we have to cast it to get a proper reference to it. (And this is not only tedious, but it also does not perform very well.) The Method class could have very easily implemented the pattern that you are describing, thus making the life of thousands of programmers easier.

Of course, this technique is only viable in small, well-defined hierarchies of closely coupled classes that you have (and will always have) source-level control of: you don't want to be doing such a thing if your class hierarchy is liable to be extended by people who are not at liberty to refactor the base class.

Here is how what I have done differs from what you have done:

  • The base class provides a default implementation for the entire asDerivedClass() family of methods, having each one of them return null.

  • Each derived class only overrides one of the asDerivedClass() methods, returning this instead of null. It does not override any of the rest, nor does it want to to know anything about them. So, no IllegalStateExceptions are thrown.

  • The base class also provides final implementations for the entire isDerivedClass() family of methods, coded as follows: return asDerivedClass() != null; This way, the number of methods that need to be overriden by derived classes is minimized.

  • I have not been using @Deprecated in this mechanism because I did not think of it. Now that you gave me the idea, I will put it to use, thanks!

C# has a related mechanism built-in via the use of the as keyword. In C# you can say DerivedClass derivedInstance = baseInstance as DerivedClass and you will get a reference to a DerivedClass if your baseInstance was of that class, or null if it was not. This (theoretically) performs better than is followed by cast, (is is the admittedly better named C# keyword for instanceof,) but the custom mechanism that we have been hand-crafting performs even better: the pair of instanceof-and-cast operations of Java, as well as the as operator of C# do not perform as fast as the single virtual method call of our custom approach.

I hereby put forth the proposition that this technique should be declared to be a pattern and that a nice name should be found for it.

Gee, thanks for the downvotes!

A summary of the controversy, to save you from the trouble of reading the comments:

People's objection appears to be that the original design was wrong, meaning that you should never have vastly different classes deriving from a common base class, or that even if you do, the code which uses such a hierarchy should never be in the position of having a base reference and needing to figure out the derived class. Therefore, they say, the self-casting mechanism proposed by this question and by my answer, which improves the use of the original design, should never have been necessary in the first place. (They don't really say anything about the self-casting mechanism itself, they only complain about the nature of designs that the mechanism is meant to be applied to.)

However, in the example above I have already shown that the creators of the java runtime did in fact choose precisely such a design for the java.lang.reflect.Member, Field, Method hierarchy, and in the comments below I also show that the creators of the C# runtime independently arrived at an equivalent design for the System.Reflection.MemberInfo, FieldInfo, MethodInfo hierarchy. So, these are two different real world scenarios which are sitting right under everyone's nose and which have demonstrably workable solutions using precisely such designs.

That's what all the following comments boil down to. The self-casting mechanism is hardly mentioned.

  • 13
    It's legitimate if you designed yourself into a box, sure. That's the problem with a lot of these types of questions. People make decisions in other areas of the design which force these type of hackish solutions when the real solution is to figure out why you ended up in this situation in the first place. A decent design won't get you here. I'm currently looking at a 200K SLOC application and I'm not seeing anywhere that we needed to do this. So it isn't just something people read in a book. Not getting in these types of situations is simply the end result of following good practices. – Dunk Apr 29 '15 at 20:51
  • 3
    @Dunk I just showed that the need for such a mechanism exists for example in the java.lang.reflection.Member hierarchy. The creators of the java runtime got in this type of situation, I don't suppose you would say they designed themselves into a box, or blame them for not following some kind of best practices? So, the point I am making here is that once you have this type of situation this mechanism is a good way to improve things. – Mike Nakis Apr 29 '15 at 20:56
  • 6
    @MikeNakis The creators of Java have made plenty of bad decisions, so that alone doesn't say much. At any rate, using a visitor would be better than doing an ad-hoc test-then-cast. – Doval Apr 29 '15 at 21:11
  • 3
    Additionally, the example is flawed. There is a Member interface, but it only serves to check the access qualifiers. Typically, you get a list of methods or properties and use that directly (without mixing them, so no List<Member> appears), so you do not need to cast. Note that Class does not provide a getMembers() method. Of course, a clueless programmer might create the List<Member>, but that would make as little sense as making it a List<Object> and adding some Integer or String to it. – SJuan76 Apr 29 '15 at 21:26
  • 5
    @MikeNakis "A lot of people do it" and "Famous person does it" are not real arguments. Antipatterns are both bad ideas and widely used by definition, yet those excuses would justify their use. Give real reasons for using some design or another. My issue with ad-hoc casting is that every user of your API has to get the casting right every single time they do it. A visitor only needs to be correct once. Hiding the casting behind some methods and returning null instead of an exception doesn't make it much better. All else being equal, the visitor is safer while doing the same job. – Doval Apr 29 '15 at 21:29

protected by gnat Apr 30 '15 at 19:39

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.