0

Feel free to improve the title and to add tags. I did not know the terminology to use here.

I have classes like this:

abstract class A {
    void foo(Object obj) {
        pre();

        if(obj instanceof Map<String, String[]>) {
            methodA();
        } else if (obj instanceof Map<String, String>) {
            methodB();
        } else if (obj instanceof JSONObject) {
            ...

        post();
    }

    abstract protected void methodA();
    abstract protected void methodB();
}

class B extends A { /* Stuff */}

Note that I have two different maps. But let's focus on the first, Map<String, String[]>. The if statment did not compile, so I tried this:

interface MyMap extends Map<String, String[]> {}

and

if(obj instanceof MyMap) {

That compiled. However, this does not seem to work.

Map<String, String[]> map = new Hashtable<>();
B b = new B();
b.foo(map);

This makes the above if statement fail, so I tried this:

MyMap map = new Hashtable<>();

But then I get this:

incompatible types: cannot infer type arguments for java.util.Hashtable<>

and with

MyMap map = new Hashtable<String, String[]>();

I get this:

incompatible types: java.util.Hashtable<java.lang.String,java.lang.String[]> cannot be converted to MyMap

What I'm trying to achieve here is that previously, I had a separate method for each object. But I don't like the code duplication, because the pre() and post() are the same for all of them. How should I solve this?

CKing
  • 14,153
  • 4
  • 39
  • 77
klutt
  • 25,535
  • 14
  • 43
  • 72
  • Type arguments are erased at runtime, so you can only check `instanceof Map`. See [this](https://stackoverflow.com/questions/1570073/java-instanceof-and-generics) and [this](https://stackoverflow.com/questions/339699/java-generics-type-erasure-when-and-what-happens) – user Oct 01 '20 at 14:57
  • @user Well, it seems to be possible to check against `MyMap` too. It does compile. – klutt Oct 01 '20 at 14:59
  • Yes, but your first `map` instance has nothing to do with `MyMap`. `MyMap` is a separate interface, not a typealias. – user Oct 01 '20 at 15:00
  • 1
    That if/else construct smells - can you make the abstract class generic? – Andrew S Oct 01 '20 at 15:01
  • @AndrewS No I cannot, and yes, it definitely feels like a smell. But I think my problem is quite clear. Otherwise, just ask. – klutt Oct 01 '20 at 15:02

2 Answers2

2

You have some misunderstandings about java.

You seem to expect that, given:

interface MyMap extends Map<String, String[]> {}

That you've made an ontological declaration: You seem to think this means:

"I henceforth decree that all things that are a Map<String, String[]> are also MyMaps."

Or possibly you think this means:

"I have created an alias; MyMap is just shorthand for Map<String, String[]>".

Neither is true.

The correct meaning is:

I created an entirely brand new interface called MyMap. Any MyMap is necessarily a Map<String, String[]>, but the reverse need not be true. Right now, other than what that Map interface demands you have as methods, I make no further demands. The only objects that are MyMaps are any objects created by writing new X(), where X is a class that explicitly has implements MyMap on it, or is a subclass of a class that did that.

And therefore, it is completely useless in trying to address your problem here.

Your second misunderstanding is generics.

Generics are primarily a figment of the compiler's imagination. They are compiler-checked comments. Seriously. Go check:

Map<?, ?> = new HashMap<String, String>();

and

Map<?, ?> = new HashMap<Integer, Integer>();

compile both, and do a binary diff on the result. Exactly the same bytes. Thus clearly proving that the <String, String> part does not survive compilation.

This is called erasure. There where generics appears in reflectively accessible stuff, such as in the return type of a method, or the parameter of a method, or some type in an extends clause, or a field's type - it is stored in the class file, but as a comment (as in, the JVM itself doesn't even look at it and cares absolutely not one iota about it). The comment is there for javac to look at, and use when compiling other code that references these fields/methods/classes.

But everywhere else? javac uses it to generate warnings, errors, and silently inject a few casts, and then it disappears.

Thus, this:

x instanceof Map<String, String>

is impossible.

You just can't check that. x knows that it is a Map. It has no idea what the generics are, though.

The solution then is to take a step back.

You had some unknown problem X, and you thought: I know! I'll write a method that takes any object, and then I'll check if the passed-in object is a Map<String, String>, and then I'll solve my unknown problem X! Except I don't know how to check for that so I'll ask on SO.

But this is a dead end. Go back and re-ask about 'unknown problem X'. I'm positive it can be solved, and quite easily at that, but not like this.

A wild stab at a possible solution for you

If you're looking to eliminate code duplication, and the duplicated code you're trying to deduplicate is the pre and post in:

public void someMethod(SomeSpecificType x) {
    pre();
    customStuffUniqueToThisType();
    post();
}

// and 20 more of these, with the same pre and post everywhere

Then perhaps consider lambdas. You could make this method:

public <T> void apply(T obj, Consumer<T> runner) {
    pre();
    runner.consume(obj);
    post();
}

and you can then call it:

Map<String, String> obj = ....;
apply(obj, o -> {
   // o is a Map<String, String>.
   // do whatever you want here.
});
rzwitserloot
  • 44,252
  • 4
  • 27
  • 37
  • Thanks for a very informative post, but I think I actually did explain what problem I'm trying to solve in the bottom of the question? – klutt Oct 01 '20 at 15:12
  • I'm basically trying to init instances of B with different objects, and I have some pre and post that is the same in all cases. – klutt Oct 01 '20 at 15:13
  • Thank you very much for your suggestion, but in my case, I don't think it's worth all the hazzle. So I accept some code duplication in this case. I upvoted your answer. – klutt Oct 02 '20 at 08:05
2

This is a common use case already solved multiple times in the object oriented world.

What you are looking for is a combination of Template Pattern, the Strategy Pattern and the Factory pattern.

  1. After reading up on the Template pattern, it should be clear that your code is somewhat already using this pattern. (Although : foo should be final)
  2. I would make class A take a type parameter (i.e abstract class A<T>). I would create a constructor in class A that takes T as input (i.e A(T t) { this.t = t})
  3. Instead of having 2 methods namely methodA and methodB, I would simply have one abstract method called doSomething(T t); I would call it only once, passing it the t instance variable. I would remove all the if-else conditions. The final implementation of foo would be foo() { pre(); doSomething(t); post() }
  4. class B can then extend A and implement doSomething(T t). Example : class B extends A<Map<String, String[]>> { doSomething(<Map<String, String[]>> t) {...do something..} }
  5. Similarly, I can create one subclass each for handling Map<String, String> and JSONObject respecrively.
  6. At this point, I have different implementations of A. I need to have a Context class that will be passed an A and it will call a.foo(). I need a Factory class that creates a Context class and passes it the right instance of a subclass A depending on some runtime input parameter.
CKing
  • 14,153
  • 4
  • 39
  • 77
  • Thank you. The reading gave me some insight, but I think I'll stick to some minor code duplication. Answer upvoted. – klutt Oct 02 '20 at 08:06
  • @klutt Thanks for your comment. I would highly recommend that you avoid code duplication and have conditionals in one single place in the application (Read : Factory). What may seem like a non-issue today can easily pose as a code-smell that becomes almost impossible to refactor when the code base grows larger.. – CKing Oct 02 '20 at 13:08