[
https://issues.apache.org/jira/browse/TINKERPOP-3155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17945157#comment-17945157
]
ASF GitHub Bot commented on TINKERPOP-3155:
-------------------------------------------
Cole-Greer commented on PR #3095:
URL: https://github.com/apache/tinkerpop/pull/3095#issuecomment-2810508088
VOTE +1
> 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
> Components: process
> Affects Versions: 3.7.3
> Reporter: Martin Häusler
> Priority: Major
>
> 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)
