generics: is this a good/bad idea; and eclipse consequences

[allen]

Hi,
Continuing with my exploration of generics, I found the following could
be useful. I wish to get some feedback as to whether this is a terrible
idea, and why. Note that this is a slightly contrived example, so don't
challenge the assumed requirement, rather criticise the implementation.

The idea is that I wish to store a potentially nested collection of
key=>values. The type of value is not constant, the map might hold a
mixture of Integer, String, Double or nested maps. Only the client code
knows the value type for a given key.

class GenericMap
{
private Map<String, Object> map = new HashMap<String, Object>( );

/**Allows the client to specify the type he expects
*back for a given id.
*Any feedback?
*/
public <E> E get( String id )
{
// required cast
return (E) map.get( id );
}

//We can put anything into the map, so no generics used here.
public void put( String id, Object obj )
{
map.put( id, obj );
}
}

// client code:
GenericMap gm = new GenericMap( );
gm.put( "id1", new GenericMap( ) );
gm.<GenericMap>get( "id1" ).put( "id2", new Integer( 5 ) );
System.out.println(
gm.<GenericMap>get( "id1" ).<Integer>get( "id2" ).intValue( ) );
gm.put( "id2", "StringValue" );
System.out.println( gm.<String>get( "id2" ) );

I know that at runtime, the same amount of casting will take place to
access a specific entry. However at compile time it can make the code
look tidier (although I guess beauty is in the eye of the beholder and
all that). There is only 1 line that will generate the unchecked
warning, and the casting is slightly less cumbersome.

Now for eclipse (3.1.0), given that I wish to implement the above code.
An advantage to the above code is that content assist should be
available as you type in the line
gm.<GenericMap>get("id1").<GenericMap>get("id2").<Integer>get("id3").intValue(
);
However it seems to struggle with giving me anything useful. Any
comments, or should I log this as a bug?

thanks
Allen

 

[Thomas Hawtin]

class GenericMap
{
private Map<String, Object> map = new HashMap<String, Object>( );

/**Allows the client to specify the type he expects
*back for a given id.
*Any feedback?
*/
public <E> E get( String id )
{
// required cast
return (E) map.get( id );
}

A terrible idea, IMO. You seem to be interested in using an alternative
syntax for casting, which just makes code obscure. You shouldn't attempt
to remove -Xlint warnings without removing the underlying reason.

The type of generic method may be inferred. So the cast, which really
should be explicit, can be removed altogether.

GenericMap map = new GenericMap();
map.put("one", 1);
Long one = map.get("one");

Oops.

Tom Hawtin

 

[Oliver Wong]

Hi,
Continuing with my exploration of generics, I found the following could
be useful. I wish to get some feedback as to whether this is a terrible
idea, and why. Note that this is a slightly contrived example, so don't
challenge the assumed requirement, rather criticise the implementation.

I'm not sure I understand what you want/don't want me to do, but given
your requirements, this is not how I'd personally implement it.

The idea is that I wish to store a potentially nested collection of
key=>values. The type of value is not constant, the map might hold a
mixture of Integer, String, Double or nested maps. Only the client code
knows the value type for a given key.

[code snipped]

Since the client code knows the value type for a given key, I'd have 4
Maps, instead of just one. A Map<Integer>, Map<Double>, Map<String> and
Map<Map>. The client code can call manipulate the correct one, thus
eliminating all casting issues.

And if really desired, you could make wrapper class to hold all four
maps, with 4 methods (e.g. getAsString()) that query the appropriate map, so
from the client's point of view, it's dealing with just one class.

- Oliver

 

[Roedy Green]

The idea is that I wish to store a potentially nested collection of
key=>values. The type of value is not constant, the map might hold a
mixture of Integer, String, Double or nested maps. Only the client code
knows the value type for a given key.

Keep in mind that generics are a compile time checking mechanism to
make sure all HAS to logically work at run time because of compile
time analysis of what you put in and take out of containers. They
don't generate any code executed at run time.

You NEED run time checks since there is nothing but grace ensuring you
fish out the same sort of object as you put in. So don't expect
generics do such checking for you.

 

[allen]

Hi,
Thanks a lot for your input. I appreciate the feedback. I understand
what you are saying, however I would like query a few things below:

A terrible idea, IMO. You seem to be interested in using an alternative
syntax for casting, which just makes code obscure. You shouldn't attempt
to remove -Xlint warnings without removing the underlying reason.

I'm not sure in this case the underlying reason can be removed, the
fact is the map is storing a mixture of types, and so casting is
necessary. Regarding making the code obscure, again, this may be in the
eye of the beholder - see the example at the end of this post.

The type of generic method may be inferred. So the cast, which really
should be explicit, can be removed altogether.

GenericMap map = new GenericMap();
map.put("one", 1);
Long one = map.get("one");

Oops.

Yes, this is a problem, but I don't believe this is any more of a
problem than this:
Map<String,Object> m = new HashMap<String,Object>( );
m.put( "one", 1 );
Long one = (Long) m.get( "one" );
Either way, the client needs to be aware of the casting going on.

Lastly, just some more example client code, comparing the two options
(using the GenericMap class in the original post versus using a
Map<String,Object>) when using a nested map.

// Using a Map<String,Object>:
Map<String, Object> map = new HashMap<String, Object>( );
map.put( "node1", new HashMap<String, Object>( ) );
// note - the following lines cause warnings
((Map<String,Object>)map.get( "node1" )).put( "one", new Integer( 1 )
);
Integer v1 = (Integer) ((Map<String,Object>)map.get( "node1" )).get(
"one" );
System.out.println( v1 );
// The following line will cause a ClassCastException:
//Long v2 = (Long) ((Map<String,Object>)map.get( "node1" )).get( "one"
);

// Using the GenericMap class from the original post
GenericMap gm = new GenericMap( );
gm.put( "node1", new GenericMap( ) );
gm.<GenericMap>get( "node1" ).put( "one", new Integer( 1 ) );
Integer v3 = gm.<GenericMap>get( "node1" ).<Integer>get( "one" );
System.out.println( v3 );
// The following line will cause a ClassCastException
//Long value = gm.<GenericMap>get( "node1" ).<Long>get( "one" );

 

[John C. Bollinger]

Hi,
Thanks a lot for your input. I appreciate the feedback. I understand
what you are saying, however I would like query a few things below:




I'm not sure in this case the underlying reason can be removed, the
fact is the map is storing a mixture of types, and so casting is
necessary.

Yes, and Tom's point was that that *should* cause an unchecked cast
warning to be emitted. I agree with him. Jumping through hoops to
avoid type safety warnings without actually providing type safety is an
exercise in inanity.

Regarding making the code obscure, again, this may be in the
eye of the beholder - see the example at the end of this post.

Yes and no. With explicit casts, any reasonably competent Java
programmer will understand the code straight away. No doubt anyone who
has studied your GenericMap class will understand the example code you
provided reasonably well, too, but anyone else, on the other hand, will
be confused. At the very least they will have to guess at what the code
means; if it is important that they understand it clearly then they will
have to look up the docs (it will be well documented, of course). That
is precisely what makes the code obscure.

[...]

 

[allen]

Hi,

Thanks for your comments. Based on feedback from this thread, I have
changed the GenericMap class to the following:

class GenericMap
{
private Map<String, Object> map = new HashMap<String, Object>( );

public <E> E get( String id, Class<E> type )
throws ClassCastException
{
Object o = map.get( id );
return type.cast( o );
}

public void put( String id, Object obj )
{
map.put( id, obj );
}
}

my client code becomes
GenericMap gm = new GenericMap( );
gm.put( "node1", new GenericMap( ) );
gm.get( "node1", GenericMap.class ).put( "one", new Integer( 1 ) );
Integer v3 = gm.get( "node1", GenericMap.class ).get( "one",
Integer.class );
System.out.println( v3 );
Number n = gm.get( "node1", GenericMap.class ).get( "one", Number.class
);
System.out.println( n );
// ignoring ClassCastExceptions for brevity

I hope this doesn't appear to be another effort to bypass type safety!
I agreed with the concerns raised about hiding the warnings, and hence
added the ClassCastException to the throws clause. The
ClassCastException will now be thrown at the relevent point rather than
being delayed as was pointed out originally. The client is responsible
for catching the ClassCastException, I guess one could throw a compile
time exception instead in order to force the client to deal with it.

Any comments?

Thanks
Allen

I'm not sure in this case the underlying reason can be removed, the
fact is the map is storing a mixture of types, and so casting is
necessary.

Yes, and Tom's point was that that *should* cause an unchecked cast
warning to be emitted. I agree with him. Jumping through hoops to
avoid type safety warnings without actually providing type safety is an
exercise in inanity.

Regarding making the code obscure, again, this may be in the
eye of the beholder - see the example at the end of this post.

Yes and no. With explicit casts, any reasonably competent Java
programmer will understand the code straight away. No doubt anyone who
has studied your GenericMap class will understand the example code you
provided reasonably well, too, but anyone else, on the other hand, will
be confused. At the very least they will have to guess at what the code
means; if it is important that they understand it clearly then they will
have to look up the docs (it will be well documented, of course). That
is precisely what makes the code obscure.

[...]