eccentricdevotion / TARDISWeepingAngels

Scary Doctor Who monsters for Spigot / Paper servers
5 stars 1 forks source link

Part of TARDIS PR #11

Closed DalekCraft2 closed 3 years ago

DalekCraft2 commented 3 years ago

See the aforementioned for details.

I managed to change the names of files in this, and I plan to do it for the two main plugins, as well.

eccentricdevotion commented 3 years ago

Hmm, I'm not happy with changing file names... TARDIS is an acronym, therefore all uppercase, imo there is no such thing as a 'Tardis' or a 'tardis'. Package names are OK lowercase as that is common convention, but I see no rule that says you can't use underscores... IMO PRs should add something useful or fix a bug, these changes are purely "cosmetic" and don't really make the plugin better in any meaningful way.

DalekCraft2 commented 3 years ago

ignore that closing and reopening

Anyway, I have several comments to make on that.

Firstly, yeah, I can add the underscores back.

Secondly, using full-caps acronyms in Java can make things look really messy. For example, when using multiple acronyms consecutively (e.g., TARDISARS in the main plugin), the acronyms blend together to make a single word.

To add to that, the plugins do not actually always use the full-caps acronyms. TardisAPI and the "Tardis" class in the main plugin are examples of that. The latter is a bit of a bad naming choice because a "TARDIS" class exists as well.

Using camelcase for acronyms makes everything easier to read.

Of course, the full-caps acronyms can still be used in comments and Javadocs, unless, for example, they refer to an object with the name of "Tardis" and not "TARDIS".

eccentricdevotion commented 3 years ago

Secondly, using full-caps acronyms in Java can make things look really messy. For example, when using multiple acronyms consecutively (e.g., TARDISARS in the main plugin), the acronyms blend together to make a single word. To add to that, the plugins do not actually always use the full-caps acronyms. TardisAPI and the "Tardis" class in the main plugin are examples of that. The latter is a bit of a bad naming choice because a "TARDIS" class exists as well. Using camelcase for acronyms makes everything easier to read. Of course, the full-caps acronyms can still be used in comments and Javadocs, unless, for example, they refer to an object with the name of "Tardis" and not "TARDIS".

Regardless of any inconsistencies or supposed messiness, I won't be merging anything with class name changes, here or for any other TARDIS plugin.

DalekCraft2 commented 3 years ago

Secondly, using full-caps acronyms in Java can make things look really messy. For example, when using multiple acronyms consecutively (e.g., TARDISARS in the main plugin), the acronyms blend together to make a single word.

To add to that, the plugins do not actually always use the full-caps acronyms. TardisAPI and the "Tardis" class in the main plugin are examples of that. The latter is a bit of a bad naming choice because a "TARDIS" class exists as well.

Using camelcase for acronyms makes everything easier to read.

Of course, the full-caps acronyms can still be used in comments and Javadocs, unless, for example, they refer to an object with the name of "Tardis" and not "TARDIS".

Regardless of any inconsistencies or supposed messiness, I won't be merging anything with class name changes, here or for any other TARDIS plugin.

What about method name changes?

And maybe package name changes, because the all-caps group ID for TARDIS does not follow conventions.

eccentricdevotion commented 3 years ago

Conventions are just that - conventions. They're like the pirates' code - the code is more what you'd call "guidelines" than actual rules.

Part of working on code is learning to live with the quirks of the guy who has been coding the plugin for the last 9 years and working with the code style of the project - it may not be the way you would do it, but that's something you have to get used to when working in collaboration with others. Contributors have come and gone on this project, as takes their fancy, but I've continued on. It may not be the prettiest code on the planet, but for the most part it works OK.

Changing class/method/variable names to meet "conventions" doesn't really add anything to the code, doesn't make the code more efficient, doesn't fix any bugs or add any new features - they just change what I'm very used to having worked on this for so long and really don't have any issues with in the first place - I code in many different languages, so Java conventions are not really that important to me.

If it's not actually broken, don't fix it. Find something that does need fixing or adding instead - if you're making a new class feel free to use whatever names you want, but generally just leave existing code/names as is.

eccentricdevotion commented 3 years ago

Closing this due to reasons as stated in previous comments, here and on other PRs in this series. Please feel free to submit parts of this as separate PRs, but please test before submitting so that only the commits necessary are included in a clean fork.