Skip to content
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

Method and field names for singletons should be unified #3574

Open
3 tasks
haykam821 opened this issue May 10, 2023 · 2 comments
Open
3 tasks

Method and field names for singletons should be unified #3574

haykam821 opened this issue May 10, 2023 · 2 comments

Comments

@haykam821
Copy link
Contributor

Classes using a singleton pattern generally store an instance of the class in a private field and provide a public method for accessing the instance. However, the names of these methods and fields are not consistent, so these names should be changed.

  • Find all classes with a singleton pattern
  • Determine which method and field names to use
  • Change the method and field names

For example, BiomePlacementModifier and SquarePlacementModifier both use an INSTANCE field and of() method.

@liach
Copy link
Contributor

liach commented May 11, 2023

of() is an alternative to a no-arg constructor for immutable classes, like List.of(). Otherwise, I would recommend getInstance() if the class is mutable, such as the fabric loader or Minecraft client instance. Thus, immutable no-arg instance should be called EMPTY or DEFAULT, while the getInstance() ones should be called INSTANCE (mutable, but it's static final field)

@apple502j
Copy link
Contributor

apple502j commented May 11, 2023

I believe, at least here, consistency within a package is more important than consistency within Yarn as whole. I do think getInstance should be used for things that must be singleton, and of should be used where it is just performance optimization (and similar classes in the package, taking arguments, have the of method.) I think other placement modifiers use of, in that case all modifiers should use of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants