Skip to content

Class naming convention #814

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

Closed
kashike opened this issue Apr 25, 2019 · 16 comments
Closed

Class naming convention #814

kashike opened this issue Apr 25, 2019 · 16 comments

Comments

@kashike
Copy link
Collaborator

kashike commented Apr 25, 2019

We're planning to do the following changes to make things consistent, and 1.14 is a good time to do it with the large changes.

  • Drop the I prefix for interfaces
  • Drop the Enum prefix for enumerations
  • Suffixes for everything: FooBlock, FooItem, etc
    • not everything with a Block prefix is actually a block (applies elsewhere too):
      • BlockMatcher is actually a predicate
      • BlockPattern is actually a pattern that can be matched in the world
      • BlockState is actually a state of a block
    • doesn't apply to vanilla, but applies to mods who don't use packages and follow vanilla's class naming: FooBlock, FooItem are both grouped together, rather than BlockFoo [... many others ...] ItemFoo
      • while vanilla separates blocks and items into their own packages, a mod is not required to do the same
    • note that we already use suffixes for major refactors and new classes: see Biome and Feature classes for an example

Deadline for feedback is Monday (29/04/2019).

@kashike kashike added this to the 1.14.0 milestone Apr 25, 2019
@kashike kashike self-assigned this Apr 25, 2019
@Shadows-of-Fire
Copy link

👎 for suffixes, the general feedback from previous questions like this has been to keep prefixes. At least for game objects (anything that goes into a registry), prefixes should remain. The only case where the prefix is not useful for matching two alike-prefixed things together is a block and it's pair itemblock.

The Enum prefix can go die in a fire, it's not implemented consistently anyway. The I prefix for interfaces is inconsistent with normal java interface names, and could be dropped without problems, but it is helpful for distinguishing what is/isnt an interface without having to look up the class.

@tterrag1098
Copy link
Collaborator

tterrag1098 commented Apr 25, 2019

Suffixes

e.g. FoodItem, OreBlock

PROs

  • No more conflicts between types, such as ItemBlock and BlockState (however new conflicts may arise)
  • Easier to search for by object ("Food" or "Ore" above) when using IDE search

CONs

  • Change of the status quo, requires a significant amount of renames

Prefixes

e.g. ItemFood, BlockOre

PROs

  • Status quo is maintained, less friction with updates and updates to documentation
  • Easier to search for by type ("Item" or "Block" above) when using IDE search as name goes from less to more specific (similar to how unloc names function).

CONs

  • Repetitive when packages are already done by the type, i.e. all item classes inside n.m.item begin with Item.
  • Existing name conflicts remain

Personally I prefer prefixes, but I feel it is necessary to lay out an objective list of pros and cons. If you have any I missed here, feel free to comment and I will add them. But reasons like "it's better" and other subjective things do not belong here.

@Sollace
Copy link

Sollace commented Apr 25, 2019

@kashike
I would still keep the I prefix on interfaces as a way to distenguish them. Otherwise you can't tell if something is a class or an interface, simillar to the confusion that can occur if you name a generic type class Foo.

@marvin-roesch
Copy link
Contributor

I've mentioned this in previous discussions and will reiterate here: neither prefixes nor suffixes are objectively better for IDE discovery. It's entirely up to what "property" of the class you're interested in for you search.

A prefix search like 'Block*' is probably better if you're not entirely certain what class a block is associated with, since you see a neatly aligned list where you can focus on the suffix.

Suffixes, on the other hand, arguably are better when you're looking for anything that is e.g. related to chests. Typing in 'Chest' would immediately yield the associated block, tile entity, GUI and container class, with the important part similarly aligned.

In summary, one might say that prefixes are favourable for "type"-related searches while suffixes benefit "object"-related searches. Hence, what you listed as pro for prefixes above is ultimately subjective and I'd request it to be either removed or changed such that it reflects the concrete search benefit. Then you should also add the pro I outlined above to suffixes, though.

@tterrag1098
Copy link
Collaborator

@PaleoCrafter I updated the list to better reflect the objective benefits.

@Commoble
Copy link

if we change prefixes to suffixes will ItemBlock be renamed BlockItem

@RCXcrafter
Copy link

I'm indifferent about the prefix suffix thing but I definitely think the interface and enum prefix should not be dropped. the prefix allows you to immediately see what kind of class something is and it's very self explanatory. The Enum prefix could be changed to E if anyone is bothered by the length.

@gigaherz
Copy link

Big 👍 from me to change to suffixes. It feels more natural from a programming point of view -- most professional settings use suffixes for this kind of thing.

Also,

Easier to search for by object
Easier to search for by type

I don't fully agree with that, it's exactly as easy to enter "block flower" as "flower block" in the symbol search, at least on IDEs capable of substring searches.

Eg, typing "Flower" in Intellij on a 1.12 project I happen to have open right now:

image

The fact the block class starts with Block is irrelevant there -- you get the same overview.

@marvin-roesch
Copy link
Contributor

@gigaherz, it is of course correct that IDEs will search for substrings, but the argument goes beyond the act of finding anything at all.

IDEs still prefer prefix matches over substrings in their search results and you don't want to walk through a long list to get to the actually desired result. Plus, I'd argue that if the top search results share a common prefix, it's a lot easier to parse the important information quickly.

So it's not so much about the search function itself, but the representation of results that differs and favours prefixes (be they "type" or "object").

@mcenderdragon
Copy link

I would also say keep the Enum and I prefixed, when reading code it is easier to know what these objects are.
About the suffix & prefix thing I am not sure, I personaly use prefixes for everything becuase I like "HelperRendering" more than "RenderHelper" as I can jsut type "Helper" and get all helper classes, but thats just personal coding style.

@ChloeDawn
Copy link

I'll be glad to see I prefix gone, it conflicts with the JDK's primary naming pattern let alone what Mojang might be doing

@Zidane
Copy link

Zidane commented Apr 25, 2019

The only value in keeping Prefixes are when you are looking for a Block but aren't sure what it is but would know it when you see it. Which is of little value in my opinion. Besides any modern IDE does exactly what @gigaherz showed above so Prefix is even more useless.

Now I/Enum prefixing ha some logic behind it. Unless you look at the object's definition you may not know exactly what it is at face value. Even so, it only takes a moment to see it and then that Prefix looses all value. So I would still axe those as well.

That way we can finally start making MCP consistent (at least the class names).

@gabizou
Copy link

gabizou commented Apr 25, 2019

Personally speaking:

  • Drop the I prefix for interfaces
  • Drop the Enum prefix for enumerations

EnumX prefixing can die in a fire. Along with IInterface prefixing. The prefixing not only is silly, but it's also overly verbose for something as simple as BlockSlab.EnumBlockHalf. We already have the understanding that these enum types are enums from IDE intelligence and to "see whether something is an enum" by looking, well, that's just mostly familiarity of the codebase at that point. Knowing whether something is an interface is also a moot point because of familiarity with the codebase. It doesn't change much if anything whether I'm referring to a BlockState versus IBlockState, they're both interfaces and abstract to the IDE, and me, the developer, I'd know when I'm writing and reading what it is because of syntax highlighting.

In short, I don't need the name to explicitly tell me that it's an interface, otherwise we should be renaming literally everything to be abstract or interfaced or enumed etc. For example, we would/should rename MinecraftServer to AbstractMinecraftServer or AMinecraftServer if we were to want to keep ICommandSender or IRegistry or EnumBlockHalf or EBlockHalf. It's silly to try to keep that sort of consistency as exceptions.

  • Suffixes for everything: FooBlock, FooItem, etc

    • not everything with a Block prefix is actually a block (applies elsewhere too):

      • BlockMatcher is actually a predicate
      • BlockPattern is actually a pattern that can be matched in the world
      • BlockState is actually a state of a block
    • doesn't apply to vanilla, but applies to mods who don't use packages and follow vanilla's class naming: FooBlock, FooItem are both grouped together, rather than BlockFoo [... many others ...] ItemFoo

      • while vanilla separates blocks and items into their own packages, a mod is not required to do the same
    • note that we already use suffixes for major refactors and new classes: see Biome and Feature classes for an example

As for preferring BlockX vs. XBlock, I'm impartial as it's still smurf naming, but it can't be avoided, otherwise we'd have to fully qualify different classes for the same thing (like net.minecraft.tileentity.Note vs net.minecraft.block.Note etc.), so in respect for the packages already defining the grouping, I'd wager it's easier to understand NoteBlock and NoteTile equally as BlockNote and TileEntityNote.

With regards to fixing the classes that are now more generic, they should be just a TypeMatcher, TypePattern, etc.

@darkevilmac
Copy link

Personally I prefer prefixes for registry items, I can understand getting rid of the IInterface and EnumEnum though. Its easy enough to figure out if something is an interface or enum just by looking at the icon your IDE puts in front of it.

Either way it looks like a majority of people would prefer to use suffix naming anyway, so my opinion isn't really going to change anything. I'll just continue using the naming I prefer in my own projects and get used to suffix naming in Minecraft itself.

@LexManos
Copy link
Member

LexManos commented Apr 25, 2019

Just a note. I am going to leave this up to the community, I've instructed @kashike to create a new issue, with each of these choices as a separate comment on that issue. So people can vote using the 👍 and 👎 reactions. As having it all in one comment is a "all or nothing" vote, which is not good. This way we have a documented source for why things were chosen.

As for my 2 cents.
EnumX: This is a hold over from back before packages/inner classes were a thing. And it made sense to group enums together as we had a giant single So getting rid of it is perfectly fine with me.

'IInterface': This, I take full blame for, it's a personal coding preference, blame the project I was working on directly before the first pass of class names being a C# one where this is a common pattern. I am perfectly fine with removing them.

Pre-vs-Post smurp: The prefix smurphing, again is a hold over from pre-package days. Where we essentially faked packages by grouping class names together with prefixes. I prefer suffix, as it gets the information I care about to me first. "I know they are Blocks they are in the block package". But I also see the status-quo argument. So I either way works for me. We'll see how the votes go.

Are there any other things that need to be brought up?

The plan is to have the deadline for votes/new class names be Monday. So I can finish the 1.14 update.
However the votes go, this will become the new standard. And all legacy names will be converted to follow it. This means large scale class renames. But we will provide a list for people who want to auto-refactor their code.

Note: Anyone who wants to help with new class names for 1.14, #mcpconfig on the Forge discord.

@kashike
Copy link
Collaborator Author

kashike commented Apr 25, 2019

Discussion and voting are moving to separate issues: #815, #816, and #817.

@ModCoderPack ModCoderPack locked and limited conversation to collaborators Apr 25, 2019
@kashike kashike closed this as completed Apr 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests