Cleanup ChannelId handling of basic methods

Motiviation:

Simplify implementation of compareTo/equals/hashCode for ChannelIds.

Modifications:

We simplfy the hashCode implementation for DefaultChannelId by not
making it random, but making it based on the underlying data. We fix the
compareTo implementation for DefaultChannelId by using lexicographic
comparison of the underlying data array. We fix the compareTo
implementation for CustomChannelId to avoid the possibility of overflow.

Result:

Cleaner code that is easier to maintain.
This commit is contained in:
Jason Tedor 2017-02-14 21:26:11 -05:00 committed by Scott Mitchell
parent 84188395be
commit d59b4840c1
4 changed files with 50 additions and 22 deletions

View File

@ -64,6 +64,20 @@ public final class MathUtil {
return (index | length | (index + length) | (capacity - (index + length))) < 0; return (index | length | (index + length) | (capacity - (index + length))) < 0;
} }
/**
* Compares two {@code int} values.
*
* @param x the first {@code int} to compare
* @param y the second {@code int} to compare
* @return the value {@code 0} if {@code x == y};
* {@code -1} if {@code x < y}; and
* {@code 1} if {@code x > y}
*/
public static int compare(final int x, final int y) {
// do not subtract for comparison, it could overflow
return x < y ? -1 : (x > y ? 1 : 0);
}
/** /**
* Compare two {@code long} values. * Compare two {@code long} values.
* @param x the first {@code long} to compare. * @param x the first {@code long} to compare.

View File

@ -178,10 +178,10 @@ public final class DefaultChannelId implements ChannelId {
// random // random
int random = ThreadLocalRandom.current().nextInt(); int random = ThreadLocalRandom.current().nextInt();
hashCode = random;
i = writeInt(i, random); i = writeInt(i, random);
assert i == data.length; assert i == data.length;
hashCode = Arrays.hashCode(data);
} }
private int writeInt(int i, int value) { private int writeInt(int i, int value) {
@ -247,21 +247,35 @@ public final class DefaultChannelId implements ChannelId {
} }
@Override @Override
public int compareTo(ChannelId o) { public int compareTo(final ChannelId o) {
return 0; if (o instanceof DefaultChannelId) {
final byte[] otherData = ((DefaultChannelId)o).data;
if (this == o) {
// short circuit
return 0;
}
// lexicographic comparison
int len1 = data.length;
int len2 = otherData.length;
int len = Math.min(len1, len2);
for (int k = 0; k < len; k++) {
byte x = data[k];
byte y = otherData[k];
if (x != y) {
// treat these as unsigned bytes for comparison
return (x & 0xff) - (y & 0xff);
}
}
return len1 - len2;
}
return asLongText().compareTo(o.asLongText());
} }
@Override @Override
public boolean equals(Object obj) { public boolean equals(Object obj) {
if (obj == this) { return this == obj || (obj instanceof DefaultChannelId && Arrays.equals(data, ((DefaultChannelId) obj).data));
return true;
}
if (!(obj instanceof DefaultChannelId)) {
return false;
}
return Arrays.equals(data, ((DefaultChannelId) obj).data);
} }
@Override @Override

View File

@ -40,7 +40,7 @@ final class EmbeddedChannelId implements ChannelId {
} }
@Override @Override
public int compareTo(ChannelId o) { public int compareTo(final ChannelId o) {
if (o instanceof EmbeddedChannelId) { if (o instanceof EmbeddedChannelId) {
return 0; return 0;
} }

View File

@ -16,22 +16,25 @@
package io.netty.channel.embedded; package io.netty.channel.embedded;
import io.netty.channel.ChannelId; import io.netty.channel.ChannelId;
import io.netty.util.internal.MathUtil;
public class CustomChannelId implements ChannelId { public class CustomChannelId implements ChannelId {
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;
private int id; private int id;
public CustomChannelId(int id) { CustomChannelId(int id) {
this.id = id; this.id = id;
} }
@Override @Override
public int compareTo(ChannelId o) { public int compareTo(final ChannelId o) {
if (o instanceof CustomChannelId) { if (o instanceof CustomChannelId) {
return id - ((CustomChannelId) o).id; return MathUtil.compare(id, ((CustomChannelId)o).id);
} }
return -1;
return asLongText().compareTo(o.asLongText());
} }
@Override @Override
@ -41,10 +44,7 @@ public class CustomChannelId implements ChannelId {
@Override @Override
public boolean equals(Object obj) { public boolean equals(Object obj) {
if (obj instanceof CustomChannelId) { return obj instanceof CustomChannelId && id == ((CustomChannelId) obj).id;
return id == ((CustomChannelId) obj).id;
}
return false;
} }
@Override @Override