dlmarion commented on code in PR #4309:
URL: https://github.com/apache/accumulo/pull/4309#discussion_r1508934819
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftTransportKey.java:
##########
@@ -81,7 +92,7 @@ public boolean equals(Object o) {
return false;
}
ThriftTransportKey ttk = (ThriftTransportKey) o;
- return server.equals(ttk.server) && timeout == ttk.timeout
+ return type.equals(ttk.type) && server.equals(ttk.server) && timeout ==
ttk.timeout
Review Comment:
> could we make ThriftClientTypes an enum? Does not seem like this would
work with the type parameter.
IIRC I tried this when I originally created ThriftClientTypes and it didn't
work with the generics
> could we make ThriftClientTypes constructor private? This makes it easier
to reason about single instances. The subclasses would probably need to be
pulled in as inner classes to do this. Not sure if its workable even then
though.
I think we could probably make it `protected` vs `public`, or remove the
modifier and make it package protected. I looked at all of the impls and their
constructors have no modifier, so they are package protected.
--
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]
