Drop "I" prefix for interfaces #816
Comments
kashike
changed the title
kashike
changed the title
|
Are there any cases where we have an interface 'ISomething' and an implementation' Something'. As this might need to be considered since it would need to be renamed as well. As for the actual argument, I would like to point out how nice it is to have the 'I' for quickly understanding code without the use of an IDE. I often review code on my work machine and phone without having an IDE to syntax highly or hint at something being an interface. This is not so much an issue for older code as I have a lot of it memorized. However, newer stuff could cause confusion that would have been saved with one extra char. |
|
I generally support this, with concern over cases where the Out of the 168 interfaces with the The complete list... a bit messy.
I propose keeping these pairs named with the interface keeping the |
|
I is a long established Java convention. I think it is short sighted to abandon it. [Edited for hyperbole] |
|
It is not used anywhere in the Java standard library, I think it's a bit hyperbolic to say this is "abandoning all convention". To everyone here, remember your milage may vary. Things that are common in libraries/convienient in tooling one person uses do not necessarily translate to those used by anyone else. Please try to keep this in mind when making points. |
|
@bs2609 the jre is actually a poor reference for many things that are otherwise widespread convention, partly because it was written prior to said convention existing. Distinguishing interface from implementation is the goal of this convention. If we remove I, we should add an Impl suffix. A convention that is in use in the JRE. |
|
Ignoring the for or against for a second. That is a large enough list that we need to talk over what to rename other classes. As we should never have two classes/interfaces named the same. Since this can play hell on auto import and other tools. It is also bad for the brain to have to remember two separate things with the same name. |
Tackling this is the important part that needs to be discussed, which was omitted entirely from the original post. The fact that that post will likely be upvoted more summarises my concerns with this exercise as it stands. |
|
Counterpoint: I believe usage of the I prefix is less than widespread, as a matter of fact my own (albeit limited) java experience and research suggests the opposite: A quick google search for "java interface naming convention" or "java interface I prefix" finds almost exclusively results against the prefix. Additionally, checking a few random popular java libraries/tools (guava, log4j, apache commons, junit, and spring) - none of them used the prefix. Side note: I think the convention of an "Impl" suffix would be a good addition to this proposed change. |
See https://google.github.io/styleguide/javaguide.html#s5.2.2-class-names; interfaces should be adjectives so that it better reflects their abilities to have multi-inheritance and their names don't conflict with noun class names. |
|
the I prefix is convention in C#, but in Java it's mostly shunned, including being prohibited by the above-linked google code conventions. Definitely in favor of removing both that and Enum |
|
I agree with dropping the prefix. I would also agree with the After running the Minecraft JAR through a program, here's some data I found. Interfaces with only one implementation that shares the same name (add an
Interfaces that have one implementation with the same name, but also have other implementations (rename the offending implementation):
Interfaces that aren't related to the class they conflict with (rename one or both):
|
|
Also of note is that there are quite a few interfaces with only one implementing class, but that don't otherwise have naming conflicts. Might be worth a look at some of them. https://hastebin.com/soruzacoso.txt |
|
This convention document says nothing about prefixes or suffixes: https://www.oracle.com/technetwork/java/codeconventions-135099.html. 😛 Here's a classic argument about Impl vs I from the oracle forums 8 years ago: https://community.oracle.com/thread/2196495 I'm sure more googling can find debates raging for years before that. I remember having a discussion with a customer (I don't care, just be consistent) in 1998 on this very topic. Since the code base adopted I years ago, I think we should just stick with it. Changing everything to Impl or removing it would be a huge amount of confusing work for zero net gain. |
|
using adjective (phrases) whenever possible would relieve this conflict and defeats the cause of an I prefix. |
|
@cpw that convention also wasn't updated or really used by anyone since 1999. Discussion from decades ago isn't exactly relevant to what conventions are in use over most Java projects today. Most modern, maintained java projects I can find (https://github.com/apache/flink, https://github.com/spring-projects/spring-boot, https://github.com/google/guice) as well as all the ones I worked on don't use the I prefix. In fact, I'd say mcp/forge is about the only Java codebase I've seen that isn't abandoned/on life support and still uses that. As for why the rename could be beneficial, it has to do with being able to find things. The only purpose of the I suffix is to let users know it's an interface. These days your IDE does that for you just fine, since the actual declaration is a hotkey away, and you'll get an error highlight immediately if you try to make an instance of it. So it outlived its usefulness and gave way to a new issue. When a person who isn't too familiar with the difference sees IBlockState and BlockState, they will likely prefer the latter. On the other hand BlockState and BlockStateImpl makes it fairly obvious that you should prefer BlockState. Since interfaces in general should be used more often than implementing classes, Impl is preferable, as it will naturally drive people to use interfaces as they navigate the codebase and will make them consider whether they actually need the implementation class. There is some debate going around on whether it's better to use Impl suffix or a prefix like Default, but Impl is good enough, imho. |
|
The premise that I was used for interfaces only because of lack of packages is completely false. The use of I as an interface prefix is very much a valid and common naming scheme. This whole change feels completely pointless and will only serve to further confuse and complicate things for existing projects. |
This is true, but the premise that I as a prefix is "common" in java projects (that are being developed/used in 2019, not 1999) is demonstrably false. The "status quo" argument is reasonable, but I personally disagree that sacrificing long term quality to avoid short term confusion (especially on an update that changes so much already) is worthwhile. |
|
The reasoning behind the names was given in #814 (comment). Talk about packages may be a copy-paste issue from #815/#817 as it wasn't mentioned in the linked comment, but @kashike can probably clarify better there. |
|
One possible avenue for preventing clashes between interfaces and their "default" implementation, while maintaining some semblance of semantics, would be to classify the implementation using some defining property. We have the While this might not be viable for all (probably most) interface-class combos, it certainly should be considered where it makes sense. The problem with singular classes being marked using an One example of a more "semantic" approach would be calling the (current) default |
|
Adding to this if you're just going to replace "I' with "Impl" its not solving the problem. Instead its just moving the issue from one side to the other. You have to look at the problem from an efficiency and effectiveness perspective. Having the 'I' creates quick mental metadata for developers to understand its an interface without looking at the package, the import, the class header, using an IDE helper/feature, etc. It effectively tells a user "I am an guide to implementing this thing" and efficiently lets a developer start to mentally map information about a problem/logic set. Then if they need to look at the documentation/methods to implement they can open the class. Rather then opening it first to see what it even is or using an IDE feature to give that same result. The other side of this is having to create renamed implementation classes. So you end up with ISomething, SomethingAbstract, SomethingImp, and SomethingSubtype. Which can both increase confusion and result in more work for the loss of 1 char. Mix this with mods having different styles you can end up with a lot of mental overhead to develop projects. Also, just like I pointed out in the other posts. This would result in a lot of renaming for developers updating. Some of us are already struggling to keep up with updates. If we throw a lot of changes onto the pile it will require update tools to get mods even 25% updated. So if we go through with all 3 proposed changes I expect there to be an update tool and a guide on how to use it. As well steps to solve problems that might happen with the conversion. Otherwise we are going to see a large influx of developers asking for help in discord with the conversion. |
|
Something to keep note of: we now have classes that look like this: public interface C_1464_cjd extends C_1892_zk<C_1463_cjc> {
C_1464_cjd field_214911_b = func_214910_a("always_true", (p_214909_0_) -> {
return C_1452_ciq.field_215190_a;
});
C_1464_cjd field_214912_c = func_214910_a("block_match", C_1454_cis::new);
C_1464_cjd field_214913_d = func_214910_a("blockstate_match", C_1455_ciu::new);
C_1464_cjd field_214914_e = func_214910_a("tag_match", C_1466_cjj::new);
C_1464_cjd field_214915_f = func_214910_a("random_block_match", C_1460_ciz::new);
C_1464_cjd field_214916_g = func_214910_a("random_blockstate_match", C_1461_cja::new);
static C_1464_cjd func_214910_a(String p_214910_0_, C_1464_cjd p_214910_1_) {
return IRegistry.func_218325_a(IRegistry.field_218363_D, p_214910_0_, p_214910_1_);
}
}This would be Most people aren't going to be familiar with referencing fields from interfaces like this when dealing with Minecraft, as they're used to having a separate "list" for fields like this, as seen by the |
ok, thanks
Could you please stop gatekeeping then? It's really unfortunate this issue seems like it won't pass, because it actually achieves what many no voters want (reducing class/interface name churn).
The information in this thread is spread out, so here's a list of reasons MCP should adopt this change that have nothing to do with personal preference (somewhat listed from most to least important):
|
|
The removal of |
...proceeds to use the word 'prettier' in one of the reasons |
Having a consistent upper camel case rather than having consecutive uppercase letters indeed is both good for standardization and for the visuals. |
I used the word subjectively, but I think that most people would agree that removing the Also, don't focus too much on the least important reason in my list when there are much betters ones right above it. |
|
Okay, fine. The I could go either way on this one, since post-J8 there's not too much difference between Interfaces and Abstract classes. |
|
But isn't Forge restricted to Java 8 because of EnumHelper or something like that? |
|
"post-J8" in this case includes Java 8 -- it's specifically referring to versions with lambdas default methods and such, which were added in 8. |
rolls eyes :P
Forge for 1.13 runs on java 10 (but not yet on 11) |
|
Dropping the prefix on its own makes no sense. Conventions are to either have the I prefix or use an adjective-form of name (e.g. class names that end in "-able"). So in the case of IBlockState, it would've been BlockStatable - definitely not "BlockState" on its own (if still an interface). IBlockState is a conventionally-incorrect use of interfaces - it's good that they changed that to an abstract since it's an object archetype rather than a specification of capability. So, given that example, Mojang renaming classes shouldn't be of any significance to this decision. They're just doing some good refactoring, same as any dev does. |
The point is that Mojang has historically switched back and forth between interfaces/abstract classes, and there is no reason to suspect they will stop doing so in the future. Therefore having a differing convention for abstract classes and interfaces causes name change churn in future versions when this happens. |
No. Just no. For reference see the entire JRE. List, Stream, Map, ProtocolFamily, Path, FileVisitor, and more. All interfaces, without I prefix or 'able' suffix. Yes, there are things like 'Callable' and 'Iterable' But those are things that make sense to be thought of as actions. Where as the others are descriptions of objects or functionality. |
|
|
|
@ConnorJC3 You're spending more time projecting that I'm gatekeeping then what I'm actually doing. If I wanted to gatekeep I would start leveraging on a lot more than I am currently. Which I have actively been avoiding as its an easy habit to fall into when arguing. However, I have made a few mistakes on this roughly based on not proofreading outside my own perspective. With the main example when I said you have not worked on larger code bases. This was meant as a joke towards how you said you're new rather than a serious jab at your experience. Something to give me a segway into my talking point. In poor taste but not designed to gatekeep.
This is actually a very good point, moving forward that line will keep blurring. This is why I pointed out we can have a mixed standard on the topic. As some interfaces will be used like abstract classes while others will be used as "ISomething -> I am something". In which you can have several interfaces on a single class that could be anything. Take the example of my IHasMass interface. Anything can have mass but never will you have a simple instance of IHasMass. It will always be an addition to an existing object.
Funny thing with conventions is that you can make your own and do whatever you want. This is why it makes no sense for this argument to keep saying we should do X because Y convention exists. We need to pick something that makes sense for the community, forge team, and MCP. As we will be using this not whoever made the original convention.
Clearly based on the vote most people don't agree. If you did another vote that asked which looks better the numbers would be rather close to the current vote. We developers do have a habit of voting based on what we like and don't like. After all, a lot of conventions, standards, and even tools are built on what we like. No one votes purely on data or metrics. If we did all of us would be coding the same exact way. |
|
See XKCD 927: Standards. |
|
An excellent solution would be to use Impl suffix for implementing classes, and just keep the interfaces without the I. |
|
Usually this problem shouldn't arise if you do naming correctly. If you have an interface Bla then it would be weird to have a class Bla that implements that interface. Typically implementations of an interface have some actual purpose that you can use to name then. I'm not a fan of adding Impl. That looks ugly IMHO. Some common examples of naming classes that implement some interface can be: DefaultBla, CommonBla, ... Or a Minecraft example: ITickable (would become Tickable if this passes). I have never seen someone extend ITickable with something that is called Tickable. |
|
Interfaces with an What would you call those classes mentioned in #816 (comment) @McJty ? |
|
This is a question of ease of reading the code and having an idea of the type of classes, functions, objects, and methods. |
I prefer something like DefaultNetHandlerLoginClient instead of NetHandlerLoginClientImpl personally. I really don't like the Impl suffix (or prefix) |
|
I'm curious why the votes for this proposal are so much different than #815 considering it's basically the same idea: trading some "metadata" for cleaner looking names. I do want to reiterate that this is not a vote on any particular resolution to the change; using "Impl" or "Abstract" or "Default" etc has not been decided. Voting should be purely on the idea of the |
|
Could be as simple as the enum prefix is longer, and enum renames will not impact other names. Using the I prefix allows for interface marking and easier names for base classes that implement said interfaces, and I is shorter than most base class name additions (default, common, impl, base, etc) |
|
I think it's worth pointing out that in 1.14 there are lots of classes where their references are stored as interfaces, even if there is only one implementation. See |
@tterrag1098 I pointed this out earlier as it is a bit ironic given the argument. I keep pushing for keeping 'I' as metadata but don't care as much for Enum. In my mind it has a lot to do with how Enums are used vs interfaces. I use an enum as a type much in the same way as primitive. You have a fixed acceptable input range with a defined meaning. However, you have a very valid point. If we are willing to let go of Enum why are we not willing to let go of 'I' for the same reason.
I agree, this is why I think we need a mixed stance on the standard. Some cases it makes perfect sense to not include the 'I' while in others it would be extremely useful. Someone else used the example of IMob to describe this rather well. You will never have a single version of IMob or even a standalone version. It will always be something added to Entity subclass. |
|
Thank you for the input, everyone. This issue is now closed for voting, and the results will be provided shortly. |
Results calculated using https://github.com/illyohs/VoteChecker |
and others
Uh oh!
There was an error while loading. Please reload this page.
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.
Do note that these are only what vanilla names will use - you're free to name your own classes in your mods however you like.
Propsed change
Iprefix for interfacesIprefix. An example of this isIBlockState, which in 1.14 became an abstract class and hence would be renamed toBlockStateno matter what we decide in this issue.Please react with 👍 if you are in favour, and 👎 if you are not in favour.
It it HIGHLY encouraged that you read all responses to this, you can change your vote at any time up until the deadline. Please take this seriously as all results are final after the deadline.
Deadline for feedback and voting is Noon PDT Monday (29/04/2019).
ref #814
The text was updated successfully, but these errors were encountered: