Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Inheritance and casting: is this good java?

Take a look at this code (from here)

abstract class EntityA {
    AssocA myA;
    abstract void meet();
}

abstract class AssocA {
    int something;
    abstract void greet();
}

class AssocAConcrete extends AssocA {
    void greet() {
        System.out.println("hello");
    }
    void salute() {
        System.out.println("I am saluting.")
    }
}

class EntityAConcrete extends EntityA {
    void meet() {
        System.out.println("I am about to meet someone");
        ((AssocAConcrete)myA).salute();
    }
}

There are two parallel inheritance trees, for a parent class and an associated class. The problem is with line 23:

((AssocAConcrete)myA).salute();

It is a pain and I have that kind of thing all over my code. Even though that line is part of the concrete implementation of Entity, I need to remind it that I want to use the concrete implementation of AssocA, AssocAConcrete.

Is there some kind of annotation to declare that relationship? Or is there a better, more colloquial Java way to express this design? Thanks!


This is in response to @Dave, because I want to put some code in...

Interesting! So the invocation would look something like this:

AssocAConcrete myAssoc = new Assoca();
EnitityA<T extends AssocA> myEntity = new EntityA<AssocAConcrete>();
myEntity.setAssoc(myAssoc);
myAssoc.salute();

Yes? That's really cool. I think I will use it!

like image 200
pitosalas Avatar asked Feb 03 '12 16:02

pitosalas


2 Answers

I would think this is a lot neater using generics...

abstract class EntityA<T extends AssocA> {

    // Basically, this means myA is at least an AssocA but possibly more...
    T myA;
    abstract void meet();
}

abstract class AssocA {
    int something;
    abstract void greet();
}

class AssocAConcrete extends AssocA {
    void greet() {
        System.out.println("hello");
    }
    void salute() {
        System.out.println("I am saluting.");
    }
}

class EntityAConcrete extends EntityA<AssocAConcrete> {
    void meet() {
        System.out.println("I am about to meet someone");
        myA.salute();
    }
}

Aside from avoiding the casting, this also makes it much easier to add different functionality in your AssocA implementations. There should always be a way to do things without using dummy implementations (ie methods that just throw "NotImplementedException") or casting. Even though it isn't always easy or worth the refactoring time to do so. In other words, no one is going to blame you for casting (well...maybe some people will but you can't please everyone).

EDIT (Notes on instantiation):

From @pitosalas' comments below...

//Won't work...can't call 'new' on abstract class AssocA
AssocAConcrete myAssoc = new Assoca();

//Instead, do this...
AssocAConcrete myAssoc = new AssocAConcrete();

And then....

// Again, won't work.  T is only declaring the type inside your class/method.
// When using it to declare a variable, you have to say EXACTLY what you're making,
// or at least something as exact as the methods you're trying to invoke
EnitityA<T extends AssocA> myEntity = new EntityA<AssocAConcrete>();

//Instead do this...
EnitityA<AssocAConcrete> myEntity = new EntityAConcrete();

// Or this...
EntityAConcrete myEntity = new EntityAConcrete();

And then this should be good...

// Assuming this is defined as `public void setAssoc(T newAssoc) {this.myA = newAssoc;}`
myEntity.setAssoc(myAssoc);
myAssoc.salute();
like image 55
Dave Avatar answered Sep 30 '22 23:09

Dave


Looks suspicious to me. There is nothing terrible about casting, but in this case, you could resolve the issue by bringing the salute method into AssocA. Subclasses of AssocA can provide their implementations; that's part of the benefit of inheritance.

What you are doing now is saying all EntityA instances have an AssocA instance, but then in your meet method you basically force the AssocA instance to be an AssocAConcrete instance. That's the part that is suspicious; why does AssocA exist if you really need an AssocAConcrete.

Another option (based on your comments) is to invoke salute in the greet method. That way, the specific subclass has specified behavior greet, defined in the superclass, and does what it wants. In this case, salute could become private or protected. Another implementation can easily do something different, like runLikeHell.

like image 26
hvgotcodes Avatar answered Oct 01 '22 01:10

hvgotcodes