ID Constants about spongeapi

[maxov]

You're seriously not okay with calling a getId() method on BlockType or ItemType, e.g. BlockType.MELON_BLOCK.getId()? This functionality is already there, and if we do follow through with your suggestion we'll have duplicate lists of the same thing. I'm not even talking about the trouble with adding such a dependency to MC constants directly in such a list of strings which could update version-to-version. At least there's some sort of encapsulation with BlockType or ItemType.

[Soren025]

@gratimax you dont get it at all. Its not that I am not ok with it, its that it cant be used.

switch (commandArgument) { case BlockTypes.IRON_ORE.getId(): }

This doesn't work, only compile time string constants are aloud when switch casing strings.

The only way to switch case the ids currently is like this, with the assumption you know the exact id for everything

switch (commandArgument) { case "iron_ore": }

now if we had constants for the string ids this works because its a static final string constant.

switch (commandArgument) { case BlockIds.IRON_ORE: }

[octylFractal]

We compile against Java 6, not Java 7. If you compile your plugins against Java 7, you will lose users.

[Soren025]

I have no source, but I recall data gathered to make a chat that a vast majority of mine craft servers use java 7...

I will try and find this if you want.

http://mcstats.org/global/ Look at java version

[Lunaphied]

I will make the point that the vast majority of plugins for Bukkit/Spigot were written using 1.7 since that's the version that Oracle gives you by default on windows.

However we will support Java 6 because we are the lowest layer and want to follow the server. You can use 1.7 in your plugins if you wish.

[octylFractal]

Enums are a no, as seen everywhere there's been an attempt to use them. Why would you need a switch instead of an if? Fallthrough is important in some cases, but I don't really see many cases where the fallthrough would be defined in a way that isn't covered by a BlockState.

[Soren025]

oops I hit the wrong button and accidentally deleted my last comment while editing something when a new comment poppeded up.

@kenzierocks If you need to check if something is 20-30 ids I much rather have a switch.

[Lunaphied]

Switch does look nice, but as http://stackoverflow.com/questions/338206/why-cant-i-switch-on-a-string explains, the String based switch is actually less efficient than an if, it's nice, but it's not necessary.

Also it's very possible that if you're very clever and only use the switch statement, you could use java 7 to compile and java 6 to run. I haven't tested that theory though.

[octylFractal]

It's a weird new bytecode thing, I don't think it's bytecode compatible.

[Soren025]

I actually compared a switch with a string vs enum and they were equal in speed when I messed with them.

Also you say that I am free to compile to java 7 but that is not the issue here, the issue here is that there are no id constant I can use as a case statement.

[Soren025]

Seriously no legitimate reason was given other than "Sponge is java 6" or "Why not use if". I posted an issue because a style of coding I prefer is not possible. Yet I come here feeling attacked in this thread...

[octylFractal]

It's not easy to update going forward. It's inefficient. It's easy to do it wrong (how many times have you forgotten a break?). Sure, fall-through is hard, but I don't see a legitimate use case here either. Why do you need 20-30 IDs grouped together? Are they all metals? Could you just do List<String> metals = ImmutableList.of(BlockType.IRON_BLOCK.getID(), BlockType.GOLD_BLOCK.getID() /* etc */); and later do if (metals.contains(argument)) in that case?

[Soren025]

@kenzierocks http://stackoverflow.com/a/22110821, just did a quick google search, thats why. switching strings is actually faster than if else, and sure I could just make a HashSet but again I much perfer a switch case in many circumstances.

[Mumfrey]

Seriously no legitimate reason was given other than "Sponge is java 6" or "Why not use if". I posted an issue because a style of coding I prefer is not possible. Yet I come here feeling attacked in this thread

You're not being attacked, it's just that what you're proposing presents a number of problems, which would seem to be themselves more of an issue than the proposed fix. If there's a good reasoning for what you're suggesting then it can be considered, but at the moment "I want to switch on this" isn't really a sufficient reasoning because there are more efficient ways of doing the same thing (as has been pointed out already) and any method of fixing this falls foul of precisely the same issue that Enums have.

[Soren025]

@Mumfrey See above, I tested it my self a while back comparing a large switch of enums with a largh switch of strings of the same names as enums. they were equal in speed.

And what problems would there be? I just cant see how the existence of various string constants could in any way cause problems

[Soren025]

TBH If I have to Ill make the constants my self, but seriously I don't understand your reasoning.

[octylFractal]

We also don't understand yours, so we're all equal here.

[octylFractal]

Also, you never answered my 'why group these IDs'. If there's no reason, it isn't a use case.

[Soren025]

@kenzierocks

Simply that, there comes times when I need to group ids in a specific way weather I am using an item in an unconventional way such as grouping items such as sticks, blaze rods, saplings and what not as weapons, to filtering out different actions for different types of blocks for example I were to punch a block.

One specific example was that bukkit didn't have a "isOre()" in Material and I had an enum for OreType out of convenience for a small ore management plugin.

public static OreType valueOf(Material material) throws IllegalArgumentException {
        switch (material) {
        case COAL_ORE: return COAL;
        case IRON_ORE: return IRON;
        case GOLD_ORE: return GOLD;
        case LAPIS_ORE: return LAPIS;
        case DIAMOND_ORE: return DIAMOND;
        case EMERALD_ORE: return EMERALD;
        case REDSTONE_ORE: return REDSTONE;
        case QUARTZ_ORE: return QUARTZ;
        default: throw new IllegalArgumentException(String.format("no ore type linked to material, %s", material));
        }
    }

[maxov]

I understand that you may have use cases for this, but as pointed out multiple times this solution creates more problems than it solves. Better solutions have also been pointed out. After a quick discussion in IRC I have found, in total, 6 project collaborators that are against this solution, which basically destroys any chance of any contribution like this making it into master. For that reason I'm locking this issue because further discussion is not really necessary.

[Mumfrey]

@Mumfrey See above, I tested it my self a while back comparing a large switch of enums with a largh switch of strings of the same names as enums. they were equal in speed.

I didn't say it had anything to do with speed. The issue with Enums when used in APIs is that the way that they're converted to bytecode actually means they can completely screw up switch statements if they're changed in the future. This is precisely because the way switch statements on enums are compiled "bakes" a snapshot of the ordinal values of the Enum at that point in time. This makes Enums unsuitable for any kind of information which is in any way likely to change in the future.

A class full of static IDs like you're proposing has the same problem, anything which is removed will break future plugins which were previously using that value, but having the value remain behind is meaningless because the item it's referencing will no longer exist.

The nature of things being more registry-like instead of wired together with magic numbers simply means you need to adapt your code style, I'm sorry but to do otherwise just locks us into the same tired, rigidly-defined and incredibly-hard-to-change ways of the past. The solution you're suggesting has code smell about it, and really just harkens to a way of thinking which simply isn't suitable for the more dynamic and extensible nature of writing plugins/mods today.

This doesn't mean you can't use this pattern yourself, there is precisely nothing whatsoever stopping you from creating a constant pool in your own plugin containing only the hard IDs you need for your own code to work which will provide exactly the same level of functionality that you desire, but without the potential problems down the line.

Please don't assume you're being attacked just because people are against your idea, that's not how we roll. It's just that when formulating an API a bigger-picture view is required and your proposed alteration has too many negaitve side effects to be viable. Don't let this discourage you from making suggestions going forward, but be prepared to accept that things aren't always as clear cut as they may seem.

[sk89q]

If you want something like isOre(), it would be ideal if you got that from Sponge because then you'd have full support of any other mods or plugins that added custom ores.

Most of the use cases for using a switch with the Material enum was to classify materials into different categories. However, that meant that the same classification code was basically duplicated in all projects. It would be better if we were able to provide that information.

If you can think of a different use case for using a switch on an enum, then it can be discussed.