[jira] [Created] (TINKERPOP-3155) Operator.addAll checks for instanceof on the wrong parameter

[Jira]

Martin Häusler created TINKERPOP-3155:
-----------------------------------------

             Summary: Operator.addAll checks for instanceof on the wrong 
parameter
                 Key: TINKERPOP-3155
                 URL: https://issues.apache.org/jira/browse/TINKERPOP-3155
             Project: TinkerPop
          Issue Type: Bug
            Reporter: Martin Häusler


Hi,

I was experimenting with "sack(...)" today and noticed a weirdness in the 
gremlin source code for Operator. Here's the "addAll" literal, as defined in 
the gremlin source code:
{code:java}
addAll {
    public Object apply(final Object a, final Object b) {
        if (null == a || null == b) {
            if (null == a && null == b)
                return null;
            else
                return null == b ? a : b;
        }

        if (a instanceof Map && b instanceof Map)
            ((Map<?,?>) a).putAll((Map) b);
        else if (a instanceof Collection && a instanceof Collection)
            ((Collection<?>) a).addAll((Collection) b);
        else
            throw new IllegalArgumentException(String.format("Objects must be 
both of Map or Collection: a=%s b=%s",
                    a.getClass().getSimpleName(), 
b.getClass().getSimpleName()));
        return a;
    }
}, {code}
 

Note the line 
{code:java}
else if (a instanceof Collection && a instanceof Collection) {code}
 

That's weird. We're checking twice if "a" is a Collection. Not only is that 
pointless, but it also ignores parameter b. Shouldn't the second part of the 
condition be:

 
{code:java}
b instanceof Collection{code}
 

... since we're downcasting b into a Collection afterwards?



--
This message was sent by Atlassian Jira
(v8.20.10#820010)