1

I am working on legacy code like below

public Map myMethod(Map arg){
      Map newMap = createMap(); //This method creates a new Map instance.
      Iterator entries = arg.entrySet().iterator();
      while (entries.hasNext()) {
          Entry thisEntry = (Entry) entries.next();
          Object key = thisEntry.getKey();
          Object value = thisEntry.getValue();

          if ( value instanceof Map){
               Map newElement = myMethod((Map)value); // Recursive call here
               newMap.put(key, newElement);
          }else if ( value instanceof String ){
               newMap.put(key, value);
          }
      }
      return newMap;
}

Obviously, I would like to adapt Generics. So I changed the method like,

public <K,V> Map<K,V> myMethod(Map<K,V> arg){
      Map<K,V> newMap = createMap(); //creates a new Map instance.
      for ( Entry<K,V> entry : arg.entrySet()){

          K key = entry.getKey();
          V value = entry.getValue();

          if ( value instanceof Map){
               V newElement = myMethod(value); // Recursive call here
               newMap.put(key, newElement);
          }else if ( value instanceof String ){
               newMap.put(key, value);
          }
      }
      return newMap;
}

My question is about the line of recursive calls. So far, I have tried

  1. V newElement = myMethod(value); -> Compile Error
  2. Map<K,V> newElement = myMethod(value); ->compile Error
  3. V newElement = (V) myMethod((Map<?,?>)value); ->Type safety warning

--------- Edited ----------

The further assumption that I could make is

  1. createMap() method could be changed to createMap(arg)
  2. Element type of arg is not bounded to a specific Java type. This is a library method so it has to be generic
4
  • 2
    Are you sure that value is a Map of the right type? Then just cast to that type and ignore the warnings. Commented Jan 6, 2016 at 22:52
  • Problem is that V can be either String or Map<K,V> recursing itself. Map<K, Object> would work, and you would need the cast (with the safety warning, nothing can be done against that) Commented Jan 6, 2016 at 23:05
  • side note: your legacy code does not compile here : Map newElement = myMethod(value); because value is an object, not a Map Commented Jan 6, 2016 at 23:06
  • @njzk2 I edited. Thanks. Commented Jan 6, 2016 at 23:11

2 Answers 2

2

The logic of your code is not type-safe, so it's not possible to avoid a type-safety warning.

You create a new map using createMap(); that method is creating a new map blindly, with no information, so the class of map it creates is not necessarily the same as the class of arg that is passed in. That means your method myMethod() returns a map that is not necessarily of the same implementing class as the one that is passed in.

In your recursive call, you pass a map that we know is an instance of V into the recursive call, and you want the thing you get back out to be a V. But that is not necessarily true. For example, what if V were TreeMap, and the thing that is returned from the recursive call is a HashMap? Then it should not be able to cast to V, and there is no way you can safely get a V from that.

Sign up to request clarification or add additional context in comments.

2 Comments

Make sense. Assuming I change createMap() to the createMap(arg) which returns a new instance of given argument, would this method be type safe and I could enforce generics?
@exiter2000: It would be safer. But even if you change the signatures of the methods, I'm not sure you can avoid unchecked casts.
-1

Your types are all messed up here.

Your method takes a

Map<K,V> 

and returns a

Map<K,V>

That means the call

V newElement = myMethod(element); 

must be

Map<K,V> newElement = myMethod(element);  //element is of type Map<K,V>

and the call

newMap.put(key, newElement);

means that newElement must also be of type V.

So you basically need String, Map, and V to all have some common super type. In the original code, Map is really

Map<Object, Object>

so the super type is Object.

I would back up, and ask yourself why you have a map that is storing both Maps and Strings. You should probably refactor them into objects that actually represent their types.

For instance, let's say it's a file system and the Map is a directory and the String is a File, then you should really create a FileObject that can be either a directory or a file, and your method signature becomes:

Map<K, FileObject> myMethod(Map<K, FileObject> map)

2 Comments

Since this method is used by other applications(over 100 calls made to this method) and it is sort of a library method, I cannot change the nature of heterogeneous elements. This is actually simplified version of the original method. The argument Map takes Integer, List, Double, Map as elements.
Then generics won't really help you here. You need to cast, and it will be unchecked and give a warning. The compiler can't know beforehand if V is really the right type.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.