You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Thomas Neidhart (JIRA)" <ji...@apache.org> on 2014/10/29 22:05:34 UTC

[jira] [Resolved] (COLLECTIONS-536) (Code style) map.size() call in MapUtils.putAll()

     [ https://issues.apache.org/jira/browse/COLLECTIONS-536?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Thomas Neidhart resolved COLLECTIONS-536.
-----------------------------------------
       Resolution: Fixed
    Fix Version/s: 4.1

Fixed in r1635303.

Thanks for the report!

> (Code style) map.size() call in MapUtils.putAll()
> -------------------------------------------------
>
>                 Key: COLLECTIONS-536
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-536
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Map
>    Affects Versions: 3.2.1, 4.0
>         Environment: Any
>            Reporter: Tagir Valeev
>            Priority: Trivial
>              Labels: performance
>             Fix For: 4.1
>
>
> In class org.apache.commons.collections(4).MapUtils there's a method putAll(final Map<K, V> map, final Object[] array) which starts with 
> {noformat}
> map.size();  // force NPE
> {noformat}
> Actually map.size() is not that harmless call for any map. For example, consider java.util.concurrent.ConcurrentHashMap size() implementation: depending on the circumstances it may take even more time than the rest of the putAll method (at least prior to JDK 8). Things are even worse for ConcurrentSkipListMap: its size() method iterates over all the map elements. Thus I'd suggest to replace this call with more conventional check like:
> {noformat}
> if(map == null) {
>     throw new NullPointerException();
> }
> {noformat}
> If you still want to save bytes, you may use {{map.getClass()}}. It's final, so it will never be overridden to do something strange and it's even used by JavaC for the same purpose (to trigger NPE). For example, if you compile and disassemble this code:
> {noformat}
> public class Outer {
>     public class Inner {}
>     public void test(Outer n) { n.new Inner(); }
> }
> {noformat}
> You will see a getClass() call which is used to trigger NPE.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)