Skip to content

Conversation

@jpountz
Copy link
Contributor

@jpountz jpountz commented Feb 13, 2017

Relates #22278

@jpountz jpountz added :Analytics/Aggregations Aggregations >test Issues or PRs that are addressing/adding tests v6.0.0-alpha1 labels Feb 13, 2017

@Override
public boolean equals(Object obj) {
return super.equals(obj) && Objects.equals(term, ((Bucket) obj).term);
Copy link
Member

Choose a reason for hiding this comment

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

This probably needs the traditional class comparison to make sure the cast doesn't blow up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The super.equals call already guarantees it.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, fine then!


@Override
protected boolean doEquals(Object obj) {
InternalMappedTerms<?,?> that = (InternalMappedTerms<?,?>) obj;
Copy link
Member

Choose a reason for hiding this comment

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

Here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is called from equals, which already does this check.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I see now. Sorry, I wasn't being careful.


@Override
protected boolean doEquals(Object obj) {
InternalTerms<?,?> that = (InternalTerms<?,?>) obj;
Copy link
Member

Choose a reason for hiding this comment

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

Here too.

Copy link
Member

Choose a reason for hiding this comment

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

And this one is also fine.


@Override
public boolean equals(Object obj) {
return super.equals(obj) && Objects.equals(term, ((Bucket) obj).term);
Copy link
Member

Choose a reason for hiding this comment

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

Again.

Copy link
Member

Choose a reason for hiding this comment

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

As is this one.


@Override
public boolean equals(Object obj) {
return super.equals(obj) && Objects.equals(termBytes, ((Bucket) obj).termBytes);
Copy link
Member

Choose a reason for hiding this comment

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

Another.

Copy link
Member

Choose a reason for hiding this comment

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

This one is fine too.

final int numBuckets = randomInt(shardSize);
Set<Double> terms = new HashSet<>();
for (int i = 0; i < numBuckets; ++i) {
double term;
Copy link
Member

Choose a reason for hiding this comment

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

randomValueOtherThanMany might make this easier to read.

@jpountz jpountz merged commit 3bd1d46 into elastic:master Feb 17, 2017
@jpountz jpountz deleted the test/terms_agg branch February 17, 2017 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Aggregations Aggregations >test Issues or PRs that are addressing/adding tests v6.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants