dubbo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cvictory (GitHub) <git...@apache.org>
Subject [GitHub] [incubator-dubbo] cvictory commented on issue #2926: Some problems with dubbo-cluster rpc merger
Date Tue, 11 Dec 2018 03:17:26 GMT
1,  The ArrayMerger uses its first element's component type to create a new Array. But some
times
if we use 
```
        class Person{};
        class Teacher extends Person{};
        Person[] a= {new Person()};
        Teacher[] b= {new Teacher()};
        Object[] result = ArrayMerger.INSTANCE.merge(a,b);
```
This will fail with "array element type mismatch"

------

It can work well, but it is not right.

As the source code said:

```
Class<?> type = others[0].getClass().getComponentType();
```

It will get the first param's type as parameter, I think  you should keep the same type for
the merge type.  And it is not used by another module, so it is internal used api.

2,  The ArrayMerger doesn't allow null object , but ListMerger and MapMerger allows user to
input
null object 

```
       MergerFactory.getMerger(List.class).merge(list1, list2, null);
```

this is not semantic consistent.

---------

I think you can commit a pr to make the ArrayMerge receive the null parameter, but it still
should be ignored when null parameter is inputed at the runtime.

3,  All the mergers doesn't check for null input and MergeFactory.getmerger could return null

if the  request class not in cache.

As above said, it is designed for internal use, so it do not have perfect logic, if you think
it need be optimized, you can try to commit a pr .

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/2926 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


Mime
View raw message