ShardResolutionStrategyImpl needs equals() and hashCode()

Description

There's basically no way to implement a cache-aware ShardResolutionStrategy until we implement equals() and hashCode() on ShardResolutionStrategyImpl.

Environment

None

Activity

Show:
Max Ross
September 13, 2008, 7:40 PM

I'm struggling to figure out what I meant with this bug. I don't see a class called ShardResolutionStrategyImpl, and I don't see why implementations of ShardResolutionStrategy would need equals() and hashCode() in order to implement a cache-aware version since there would still only be one ShardResolutionStrategy - no need to hash or compare multiple instances. My guess is that I was actually referring to ShardResolutionStrategyDataImpl, to which I added hashCode() and equals() methods on Feb. 13. One would need hashCode() and equals() to implement a cache-aware ShardResolutionStrategy. I'm guessing I just got mixed up. Let me know if that makes sense to you, and if so I'll mark this bug fixed.

Thanks!

Max

Danny Antonetti
September 16, 2008, 5:16 AM

That makes a lot more sense.

I have the following comment about ShardResolutionStrategyDataImpl.

If you subclass ShardResolutionStrategyDataImpl then equals could break

This is the offending line code
getClass() != o.getClass()

But this is only if you ever try to compare two different implementations of ShardResolutionStrategyData

Maybe this is a moot point, because you may not ever need more than one implementation of this class in a JVM.

Max Ross
September 16, 2008, 5:52 PM

I'm ok with how equals() is currently implemented, but thanks for bringing it up.

There are certainly cases where it makes sense to treat 2 objects as equal even though they are not instances of the same class, but I don't think this is one of them. A persistent entity is uniquely identified by its name and its id, so I'm not sure why someone would need to extend this class. Maybe we should just make the class final? We're still in beta so I'm okay with this, and if anyone hollers at least we'll learn about their use case. What do you think?

Danny Antonetti
September 16, 2008, 6:37 PM

Thats fine, I just wanted to point that out, probably just too much "Effective Java" , so it stuck out a little.

I think you can close this bug.

Max Ross
September 16, 2008, 6:41 PM

I'll close the bug, but go ahead and make the class final and send me the CL.

Fixed

Assignee

Max Ross

Reporter

Max Ross

Labels

None

Feedback Requested

None

Feedback Requested By

None

backPortable

None

Suitable for new contributors

None

Pull Request

None

backportDecision

None

backportReEvaluate

None

Components

Fix versions

Priority

Minor
Configure