Friday, December 02, 2011

Coding Tip of the Day: Using Map<?,?> Collections

I was working on some code in the last few days where I came across a protected Map map instance variable. I examined the code and javadocs which indicated that the intent was to provide a mapping of keys and values. However the actual implementation was setting the keys and values to the same value. This again is not really an issue. However, the usage of the Map was incorrect.

See if you can determine the issue from the code below.
?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
...
 
private static MapValueHolder<String, String> me;
 
    public static void main(String[] args) {
 
        me = new MapValueHolderImpl<String, String>();
        Map<String, String> mx = new HashMap<String, String>();
        mx.put("A", "A");
        mx.put("B", "B");
        mx.put("C", "C");
        me.setMap(mx);
 
 
        for (String key : me.getMap().keySet()) {
            System.out.println("MapValueHolder Key --> " + key);
        }
 
        for (String key : me.getMap().keySet()) {
            System.out.println(key + " --> " + me.getMap().get(key));
        }
...
The MapValueHolder<K,V> looks like the following.
?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
/*
 * Copyright 2011 John Yeary <jyeary@bluelotussoftware.com>.
 * Copyright 2011 Bluelotus Software, LLC.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package com.bluelotussoftware.example;
 
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
 
/**
 *
 * @author John Yeary <jyeary@bluelotussoftware.com>
 * @version 1.0
 */
public abstract class MapValueHolder<K, V> {
 
    protected Map<K, V> map = new HashMap<K, V>();
 
    public MapValueHolder() {
    }
 
    public Map<K, V> getMap() {
        return Collections.unmodifiableMap(map);
    }
 
    public void setMap(Map<K, V> map) {
        this.map = new HashMap<K, V>(map);
    }
}
Here is the implementation.
?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
/*
 * Copyright 2011 John Yeary <jyeary@bluelotussoftware.com>.
 * Copyright 2011 Bluelotus Software, LLC.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package com.bluelotussoftware.example;
 
/**
 *
 * @author John Yeary <jyeary@bluelotussoftware.com>
 * @version 1.0
 */
public class MapValueHolderImpl<K, V> extends MapValueHolder<K, V> {
}

As a general rule, you should use the key to fetch the value. Even if you expect that the values are the same. Since the instance variable is protected it can be modified in a sub-class, or any class in the same package. Since we code to interfaces, or abstractions (super classes). We can not be sure that the implementation class we are provided does what we expect. In this case, it does what we expect. 
MapValueHolder Key --> A
MapValueHolder Key --> B
MapValueHolder Key --> C
A --> A
B --> B
C --> C


Here we continue using another implementation.

?
1
2
3
4
5
6
7
8
9
10
11
12
13
...
        me = new KeyValueHolder<String, String>();
        me.setMap(mx);
 
 
        for (String key : me.getMap().keySet()) {
            System.out.println("MapValueHolder Key --> " + key);
        }
 
        for (String key : me.getMap().keySet()) {
            System.out.println(key + " --> " + me.getMap().get(key));
        }
...


This time the result looks different.




MapValueHolder Key --> A
MapValueHolder Key --> B
MapValueHolder Key --> C
A --> null
B --> null
C --> null




This is because our MapValueHolder<K,V> is different.

?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
/*
 * Copyright 2011 John Yeary <jyeary@bluelotussoftware.com>.
 * Copyright 2011 Bluelotus Software, LLC.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package com.bluelotussoftware.example;
 
import java.util.HashMap;
import java.util.Map;
 
/**
 *
 * @author John Yeary <jyeary@bluelotussoftware.com>
 * @version 1.0
 */
public class KeyValueHolder<K, V> extends MapValueHolder<K, V> {
 
    public KeyValueHolder() {
    }
 
    @Override
    public void setMap(Map<K, V> map) {
        Map<K, V> mx = new HashMap<K, V>(map);
        for (K k : map.keySet()) {
            mx.put(k, null);
        }
        this.map = mx;
    }
}


Admittedly, the implementation above is a bit contrived. We really are getting is a Set<K>. In the next example, we get another result.

?
1
2
3
4
5
6
7
8
...
        me = new SingletonMapValueHolder<String, String>();
        me.setMap(mx);
 
        for (String key : me.getMap().keySet()) {
            System.out.println("MapValueHolder Key --> " + key);
        }
...


The result is considerably different.





MapValueHolder Key --> A
A --> A




Here is the implementation.

?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
/*
 * Copyright 2011 John Yeary <jyeary@bluelotussoftware.com>.
 * Copyright 2011 Bluelotus Software, LLC.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package com.bluelotussoftware.example;
 
import java.util.Collections;
import java.util.Map;
 
/**
 *
 * @author John Yeary <jyeary@bluelotussoftware.com>
 * @version 1.0
 */
public class SingletonMapValueHolder<K, V> extends MapValueHolder<K, V> {
 
    public SingletonMapValueHolder() {
    }
 
    @Override
    public void setMap(Map<K, V> map) {
        this.map = Collections.singletonMap(
                map.entrySet().iterator().next().getKey(),
                map.entrySet().iterator().next().getValue());
    }
}

Again, this is a contrived result where we are just returning the first key and value in a Map<K,V>.

Here is another example which demonstrates unexpected results.

?
1
System.out.println(new Mod().MAP + " --> " + me.map.toString());


This results in this very strange result.





Map --> {}




Here I use reflection to modify the expected result.

?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
/*
 * Copyright 2011 John Yeary <jyeary@bluelotussoftware.com>.
 * Copyright 2011 Bluelotus Software, LLC.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package com.bluelotussoftware.example;
 
import java.lang.reflect.Field;
import java.util.logging.Level;
import java.util.logging.Logger;
 
/**
 *
 * @author John Yeary <jyeary@bluelotussoftware.com>
 * @version 1.0
 */
public final class Mod {
 
    public static final String MAP = "Map";
 
    static {
        try {
            Field[] fx = App.class.newInstance().getClass().getDeclaredFields();
            Field f = fx[0];
            f.setAccessible(true);
            MapValueHolder<String, String> mvh = new MapValueHolderImpl<String, String>();
            f.set(null, mvh);
        } catch (Throwable ex) {
            Logger.getLogger(Mod.class.getName()).log(Level.SEVERE, null, ex);
        }
    }
}


If you have followed me to this point, you will have learned a little bit about the Java Type system, reflection, abstraction, and Collections. Yet, you may be asking what is the tip?

Here are some rules:



    

  1. Use the keys to fetch values.
    2. 

  2. Check for null values. In my examples, I did not check for null values.
    3. 

  3. Use the correct Java Collection.
    4. 


The Java Collection Framework covers most common cases. Take the time to read about them. The code above was based on a couple of issues I discovered: (1) Using keys for values, and (2) not using the correct collection. In this case, they should have used a List<V>. If they wanted to use an ordered Map<K,V>, they should have used a LinkedHashmap<K,V>. 




The code for the examples are here: bad-mapping.implementation.zip



The code was developed using NetBeans 7.1 RC1.































Labels:


                                ,
                              


                                ,
                              


                                ,
                              


                                ,
                              


                                ,
                              















0
comments
                          :
                        









Post a Comment
















Subscribe to:

Post Comments
                        (
                        Atom
                        )
                      








Popular Posts