sandynz commented on pull request #16036:
URL: https://github.com/apache/shardingsphere/pull/16036#issuecomment-1066443688
> How about use Map to instead of Pair?
I've investigated on this topic. It's possible to use Map instead of Pair,
but it might has some side effects. Details as following.
Since there's
```
for (Entry<Class<? extends YamlRuleConfiguration>,
YamlRuleConfiguration> entry : targetRulesMap.entrySet()) {
if (!sourceRulesMap.containsKey(entry.getKey())) {
result.add(Pair.of(null, entry.getValue()));
}
}
```
There might be several key elements are `null`, so we need to put `null` as
value and use value elements as map key else some value elements will be
replaced, e.g. `map.put(entry.getValue(), null)`.
Considering the scaling scenario, the possible side effects if we change
`Collection<Pair>` to `Map`:
1. `createJobConfig` just need a collection or array to iterate, no `get`,
it doesn't really need hash or tree map. According to `Development Guidelines`
readability, using `Collection<Pair>` is better for code purpose than using
`Map`.
2. For `boolean isRuleAltered(YamlRuleConfiguration sourceRuleConfig,
YamlRuleConfiguration targetRuleConfig)` method, the first and second
parameter's meaning is defined.
- 1) When using `Map`, target rule config might be transferred to
`sourceRuleConfig`, which is confusing.
- 2) Currently, `isRuleAltered` method just do rough comparison of old
version rule and new version rule, but we need to do detailed comparison later,
some rule change doesn't mean scaling job is necessary. Transfer target rule
config to `sourceRuleConfig` might cause issue, e.g. if dataSources is not
changed, cleanup sharding rule might not need to do scaling, but add sharding
rule need to do scaling. So when we exchange `sourceRuleConfig` and
`targetRuleConfig`, they're not equivalent.
- 3) It's possible to keep parameters ordering when invoking `isRuleAltered`
method, but it make code more complicated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
