FabricMC / yarn

Libre Minecraft mappings, free to use for everyone. No exceptions.
Creative Commons Zero v1.0 Universal
908 stars 376 forks source link

Method and field names for singletons should be unified #3574

Open haykam821 opened 1 year ago

haykam821 commented 1 year ago

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.

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

liach commented 1 year ago

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 commented 1 year ago

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.