yarn icon indicating copy to clipboard operation
yarn copied to clipboard

Clarification on `UPPER_SNAKE_CASE` convention

Open enbrain opened this issue 3 years ago • 3 comments

Currently, CONVENTIONS.md says:

Use lowerCamelCase for method names, variable names, and names of fields that are not both static and final. Use UPPER_SNAKE_CASE for names of fields that are both static and final.

However, there are fields that are used as constants but not final. Some examples are (copied from https://github.com/FabricMC/yarn/issues/2590#issuecomment-882129231):

  • net.minecraft.util.math.Vec3f.POSITIVE_X
  • net.minecraft.util.math.Vec3f.POSITIVE_Y
  • net.minecraft.util.math.Vec3f.POSITIVE_Z
  • net.minecraft.util.math.Vec3f.ZERO
  • net.minecraft.util.math.Vec3f.NEGATIVE_X
  • net.minecraft.util.math.Vec3f.NEGATIVE_Y
  • net.minecraft.util.math.Vec3f.NEGATIVE_Z

There is also a case that one person named a non-final constant INSTANCE, and the other person named it instance. See https://github.com/FabricMC/yarn/pull/2732#discussion_r725673345 and https://github.com/FabricMC/yarn/pull/2733#discussion_r725472485 for full context.

Do we allow non-final UPPER_SNAKE_CASE? In either case, a clarification in CONVENTIONS.md is needed.

enbrain avatar Oct 10 '21 18:10 enbrain

We for sure should not allow upper snake case for non-final fields as they can be set to a new value.

liach avatar Oct 10 '21 23:10 liach

We for sure should not allow upper snake case for non-final fields as they can be set to a new value.

While I agree with this in general, I still think the constant in TrueBlockPredicate should be named INSTANCE. It's very clearly used for the singleton pattern. It can be set to a new value, but that other value would still be the same as before, as there are no fields to be changed. Also, Mojang will probably make it static once they see it isn't, which would mean the name would have to be changed again.

mschae23 avatar Oct 11 '21 09:10 mschae23

lol no, the convention is based on field access modifier than it's behavior.

liach avatar Oct 11 '21 12:10 liach

The Vec3f class has been removed, so its fields no longer need to be considered. As of Minecraft snapshot 23w35a, the AlwaysTrueBlockPredicate#instance field is still not static.

I think that the conventions already do well to explain that UPPER_SNAKE_CASE is reserved for names of members that have both the static and final modifiers. The only ambiguity I can see is that the rule could also include methods with both the static and final modifiers rather than only fields.

haykam821 avatar Aug 30 '23 16:08 haykam821