FabricMC / yarn

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

`of` vs `create` for static factory methods #2492

Open Juuxel opened 3 years ago

Juuxel commented 3 years ago

Both of these are widely used throughout Yarn, but it is still very inconsistent. For example, the 1.16 class UniformIntDistribution had of, but its 1.17 rewritten equivalent UniformIntProvider has create, which is annoying for modders porting their code (no migrateMappings here either as the classes are not matched together at all).

It'd probably be better if Yarn had a consistent choice such as of, which is widely used in recent JDK additions (Set.of etc), as well as many other names also in Yarn.

Technici4n commented 3 years ago

My 0.02$: of suggests a trivial operation, whereas create suggests a complex operation.

For example, List.of is just a convenient helper.

In Fabric API, I used create for BlockApiCache because it has to be registered at object creation time for some complex behind the scenes logic.

So I expect of to be a simple conversion or wrapper, and create to be a complex operation. Do also note that create can be suitable for 0 args, but of is not - not very important though I think.

Juuxel commented 3 years ago

There's in fact a 0-arg of already in Yarn, DefaultedList.of: https://github.com/FabricMC/yarn/blob/1d4e5813b5c4d85eb9e4f3b2926dcfaa5da7a20e/mappings/net/minecraft/util/collection/DefaultedList.mapping#L12

I do agree that it should probably be changed (and in this case, given a more descriptive name to distinguish it from the other creation methods).

Technici4n commented 3 years ago

Yes, DefaultedList.empty() might be more suitable here. Or it could be seen as the equivalent of using List.of() with no arguments... But in the latter case I think I prefer Collections.emptyList().

sfPlayer1 commented 3 years ago

To me the of is the more specialized one which strongly suggests contents (or the lack thereof = emptiness with 0-arg = List of nothing), as appropriate for e.g. collections, while create conveys less information but works in other cases. I'd use whichever fits the situation best, including potential other terms.

A preposition such as of should allow to form a reasonable sentence, UniformIntProvider of <uniform ints> isn't one of those because it repeats itself, neither is it a provider of the min/max, but configured with them. List of <the supplied objects> works.

The empty List.of() is imo quite a stretch if viewed in isolation (List of <nothing> is meh), but in that specific case it naturally bridges to the var-args List.of(T...) and other overloads.

liach commented 3 years ago

Imo, create conveys that a new instance is always returned, while of can sometimes reuse existing instances for immutable objects.