Skip to content

Conversation

wujimin
Copy link
Contributor

@wujimin wujimin commented Nov 15, 2017

details is in https://servicecomb.atlassian.net/browse/JAV-479

in this issue, create new filter mechanism, old filters will rewrite in new Filter later.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 86.778% when pulling 7b30133 on wujimin:discovery-optimize into 8be387a on ServiceComb:master.

context.setCurrentNode(parent);

DiscoveryTreeNode child = filter.discovery(context, parent);
if (child == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If child is null, will generate a NullPointerException, it's the same

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but from null exception, we can not know which filter throw it.

return data == null;
}

private boolean isMapEmpty() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the data is not a map , we will get the class case exception.
I don't like these three method.

Copy link
Contributor Author

@wujimin wujimin Nov 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, i don't like this too.
but data in cache is not always the same type.
eg: in discoveryTree, before endpointFilter, discoveryTreeNode data is Map<String, MicroserviceInstance>, after endpointFilter, discoverTreeNode data is Collection<Endpoint> or Collection<CseServer>

caller know what's data type in cache, and call different method, if invoke wrong method, then it's a bug, developers should fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isXXXEmpty methods have no problems, because they are private, and used by data type.
not so good methods are: mapData/collectionData/arrayData

@liubao68 liubao68 merged commit b0cc025 into apache:master Nov 16, 2017
@wujimin wujimin deleted the discovery-optimize branch January 1, 2018 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants