-
-
Notifications
You must be signed in to change notification settings - Fork 344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rename methods that don't do well without "get" to "findX/findXOrCreate" #2332
Conversation
@@ -75,15 +75,15 @@ | |||
* @return The profile, if present, or {@link Optional#empty()} if | |||
* the cache did not contain a profile with the provided id | |||
*/ | |||
Optional<GameProfile> byId(UUID uniqueId); | |||
Optional<GameProfile> findById(UUID uniqueId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally wanted to drop "byId" etc. but the problem is that findByIds and findByNames have the same erasure so wouldn't work out so well.
We could have find
for the singular returns, and keep the current names for the... well, current values!
@@ -69,7 +69,7 @@ | |||
* @param name The state property name | |||
* @return The state property, if available | |||
*/ | |||
Optional<StateProperty<?>> statePropertyByName(String name); | |||
Optional<StateProperty<?>> findStateProperty(String name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could just be findProperty()
instead?
There is the additional complication that there are other stateProperty
methods that also return Optional
s, but they are more tied into getting a property from a key. It's less of a search than using a name is. Is there a differentiation here, or should all of these methods be findStateProperty
?
I can see the argument for saying "find" is for use by a free form identifier, lack of find when passing some key like object in, but we should decide this now.
a9877f0
to
31c174a
Compare
SpongeAPI | Sponge
See #2320. Basically here to ensure that we're okay with the proposed renames.