8

I'm still getting used to Java Generics however I'm currently in the process of updating an application written prior to generics to use the latest version of java.

Problem is the code was not written with type safety in mind!

We have a whole bunch of Maps that basically hold various object types including strings. For example:

Map map = new HashMap();
map.put("key1", "String1");
map.put("key2", new Date());
map.put("key3", new CutsomClass());

Now I'm still struggling with the best way to handle these without getting into refactoring a whole lot of code. Refactoring is not an option at this time.

Currently I can't see anything past Map<String, Object> although Map<String, ? super Object> works but I think its essentially the same thing ?

GhostCat
  • 137,827
  • 25
  • 176
  • 248
  • 8
    `Map map = new HashMap<>();` – Michael May 19 '17 at 13:43
  • 1
    `Map map = new HashMap<>()` if the key and value both hold various object types. – Ramesh KC May 19 '17 at 13:48
  • Thanks chaps... My original post missed a bit of code. That's what I've essentially been doing... Just didn't feel 'right' – Dangerous Hamster May 19 '17 at 13:50
  • 4
    Looks like bad design to me. Instead of a map, this should be a new class, with fields for the various items. – RealSkeptic May 19 '17 at 13:50
  • @RameshKC, how does Map provide any kind of type safety compared to vanilla Map for OP? I think Michael is spot on as it's half way there for the OP use case. – Imran Saeed May 19 '17 at 13:51
  • @DangerousHamster this feels like a list of "settings" or other various constants. I recommend keeping these out of a hashmap, and storing them in individual fields. That way, you not only have type checking, but you also have variable name checking (you know at compile time if "mySetting" isn't in the hashmap). – Nathan Merrill May 19 '17 at 19:44

7 Answers7

11

I'm still struggling with the best way to handle these without getting into refactoring a whole lot of code

So don't change them at all. The raw types - that is, the non-generic types - are still technically valid. It's not ideal and it will generate a compiler warning but the code will work (well, work as well as it ever did).

All classes extend Object so you can put any value you want into the following map:

Map<String, Object> map = new HashMap<>();

You get an additional guarantee that the key is a string, so its somewhat better than using the raw type.

Basically though, you should really try to avoid using a map if you can't define the type of the key or the value.

Michael
  • 41,989
  • 11
  • 82
  • 128
  • Also, the code doesn't have to be migrated all at once because there's an implicit conversion between `Map` and `Map`. This is the rare benefit of type erasure. – Radiodef May 19 '17 at 13:53
  • @Radiodef Backwards compatibility was pretty much *the* reason for implementing generics using type erasure, right? – Michael May 19 '17 at 13:54
  • 1
    @Michael yes. With all its (dis-)advantages. – Turing85 May 19 '17 at 13:55
  • 1
    Thanks @Michael that was pretty much my understand and how I've currently implemented I just wasn't sure if there was a better way of doing it... Refactoring the maps to use a better solution would be ideal but there are hundreds of these at the minute given this code is near enough 15 years old ! – Dangerous Hamster May 19 '17 at 13:55
  • 1
    In fact, it's migration specifically, since there could have been ways for them to implement reified types which would have been technically backwards-compatible. See the italicized note in [section 4.7](https://docs.oracle.com/javase/specs/jls/se8/html/jls-4.html#jls-4.7) for a more extended discussion. The fact that OP can just change the declaration of the `Map` and have the program compile is pretty much exactly the reason that they chose type erasure. – Radiodef May 19 '17 at 13:58
  • 2
    @Radiodef Good one. [Project Valhalla](https://en.wikipedia.org/wiki/Project_Valhalla_(Java_language)) is definitely an interesting proposition. – Michael May 19 '17 at 14:03
  • @DangerousHamster Never worry too much about overhauling an entire project. Always leave code tidier than you found it and soon enough all those small incremental changes will add up. – Michael May 19 '17 at 14:05
  • 1
    @Radiodef, "... there's an implicit conversion between `Map` and `Map`" No, type erasure (not a conversion) goes to `Map`, an equivalent of `Map`. The `String` type parameter is erased also. – Lew Bloch May 19 '17 at 15:55
  • @LewBloch It's a conversion. https://docs.oracle.com/javase/specs/jls/se8/html/jls-5.html#jls-5.1.9 – Radiodef May 19 '17 at 16:22
  • 1
    That is not a section dealing with type erasure; it is about a compile-time assignment or cast between raw or rare types and generic types. Type erasure is not a conversion. The section you cited does not apply here. This also does not change the fact that type erasure does make `Map` equivalent to `Map` as you claimed. – Lew Bloch May 19 '17 at 17:09
  • 1
    The section you cited talks about explicit conversions, not implicit conversions. – Lew Bloch May 19 '17 at 17:17
2

As Michael suggested Map<String, Object> map = new HashMap<>(); is a first step.
However, it assumes that you have only String as keys and you will probably need to cast objects you get from the map.

I think that a second step would be to split this map into multiple maps declaring a more specific type :

Map<String, Date> mapDates = new HashMap<>(); 
Map<String, String> mapStrings = new HashMap<>(); 
Map<String, CustomClass> mapCutsomClasses = new HashMap<>(); 
davidxxx
  • 125,838
  • 23
  • 214
  • 215
2

As of now, you can only replace the raw type Map with Map<String, Object>; but that type information is close to "useless". Unless you refactor your whole component to deal with different map objects, there isn't much you can do. Of course, you can get rid of the type warnings, but you still have to do instanceof checks each time you access a Map value.

On way out of this: assuming that number of "value" types is known and reasonably small, you could create a bunch of helper methods that go like:

public Map<String, Date> extractDates(Map<String, Object> allValues) {
...

This method could implement a "wrapper" around the allValues map that only provides those map entries that are actually Date objects.

Meaning: you keep your current map object, but you provide "more specialized" views on that map. That allows you to write new code exploiting the additional type information.

But of course, this doesn't come for free. It adds certain complexity, and defining the exact details of such "view maps" might turn out to be rather complicated.

GhostCat
  • 137,827
  • 25
  • 176
  • 248
2

There is little you can do to achieve the full static type checking available with Generics used properly. However, I don't believe you must abandon type checking completely and rely on run-time casting in this case. I think you can go one step towards that.

I assume this is a common map that the code uses for general storage, perhaps for persistence or properties etc. If this is the case then you can at least do something like this:

class AnyMap<K>  {
    final Map<K,Object> map;

    public AnyMap(Map<K, Object> map) {
        this.map = map;
    }

    public <V> Map<K,V> as(Class<V> theClass) {
        return (Map<K,V>) map;
    }
}

public void test() {
    AnyMap<String> commonMap = new AnyMap<>(Collections.EMPTY_MAP);

    // Use this one as a Date map.
    Map<String,Date> dateMap = commonMap.as(Date.class);
    // This one as a String map.
    Map<String,String> stringMap = commonMap.as(String.class);

}

This is a kind of Map holder that can then deliver the map as a proper generic object with the correct bounds. Hopefully you will find that certain modules will use the common map exclusively for Dates and others will use Strings. In areas such as these you can use Map<X,Y> as(...) to give you a properly statically-checked map for that module/section and use that exclusively in that code section.

OldCurmudgeon
  • 64,482
  • 16
  • 119
  • 213
1

Dont use raw types... see this to know why..

Now, you can break it out as following, so you can get it:

your map<K,V> has as keys Strings only, so K = string will be correct, on the other hand "String1", new Date() and new Custom Class seems to have nothing in common, but wait, all the classes in java are actually inheriting the Object class... that means you can do V=Object

now your map can be declared as Map<String, Object> and all this

map.put("key1", "String1");
map.put("key2", new Date());
map.put("key3", new CutsomClass());

will be ok

Community
  • 1
  • 1
ΦXocę 웃 Пepeúpa ツ
  • 47,427
  • 17
  • 69
  • 97
1

Conversion from non-generics (e.g. old Java) to generics can be a real PITA. The easiest way to do it is to replace each Map, Set, List with related generic, e.g.

Map<String, Object> map = new HashMap<>();

but only if mapped objects are not generics too (or are generics just used for reading). E.g. if in your code had something like

Map hasmap = new HashMap();
hashmap.put("blah",123)
map.put("keyX",hashmap);

In such cases, when you find map getter, and you will have in original code a put, you will have lot of troubles:

Map hashmap = (HashMap)(map.get("keyX"));
Integer value = hashmap.get("blah");
hashmap.put("otherkey","mooo");

In this case, you can't have clear code: if you use question marks, you will face errors, like in

Map<?,?> hashmap = (HashMap<?,?>)(map.get("keyX"));
Integer value = (Integer)hashmap.get("blah"); // this works 
hashmap.put("otherkey","mooo"); // this crashes

so you have two alternatives: rewrite code (to avoid warnings), or force everything at Objects, and receive Unchecked Casts warnings.

@SuppressWarnings("unchecked")
Map<Object,Object> hashmap = (HashMap<Object,Object>)(map.get("keyX"));
Integer value = (Integer)hashmap.get("blah"); // this works 
hashmap.put("otherkey","mooo"); // this works too

Further details about question marks and generics can be found here: What is the difference between ? and Object in Java generics?

Community
  • 1
  • 1
Sampisa
  • 1,487
  • 2
  • 20
  • 28
0

Looks like you want to quickly port the old code in and also you want to move the old code towards strict type safety without refactoring a large code base. Keep your old code by porting it in using this:

Map<String, Object> oldMapPorted = new HashMap<>();

New code written in this app can use a technique like this for strict type safety:

Map<String, Date> newMapDates = new HashMap<>(); 
Map<String, String> newMapStrings = new HashMap<>(); 
Map<String, CustomClass> newCutsomClasses = new HashMap<>();

A new class can be created for future edits and enhancements while the old code still has the same potential instability as usual.