lombok, removed hardcoded elseifelseifelseif - #5
Conversation
| if (name.contains("phase1")) | ||
| { | ||
| return lang + " (Phase 1)"; | ||
| } | ||
| else if (name.contains("phase2")) | ||
| { | ||
| return lang + " (Phase 2)"; | ||
| } | ||
| else if (name.contains("phase3")) | ||
| { | ||
| return lang + " (Phase 3)"; | ||
| } | ||
| else if (name.contains("phase4")) | ||
| { | ||
| return lang + " (Phase 4)"; | ||
| } | ||
| else if (name.contains("emerald_marbleized")) | ||
| { | ||
| return lang + " (Emerald)"; | ||
| } | ||
| else if (name.contains("ruby_marbleized")) | ||
| { | ||
| return lang + " (Ruby)"; | ||
| } | ||
| else if (name.contains("sapphire_marbleized")) | ||
| { | ||
| return lang + " (Sapphire)"; | ||
| } | ||
| else if (name.contains("blackpearl_marbleized")) | ||
| { | ||
| if (name.contains("phase)) { | ||
| return lang + " (" + capitalize(name.substring(0, 1)) + " " + name.charAt(5) + ")"; | ||
| } else if (name.contains("marbleized") && !name.contains("blackpearl")) { | ||
| return lang + " (" + capitalize(name.replace("marbleized", "")) + ")"; | ||
| } else if (name.contains("blackpearl_marbleized")) { |
There was a problem hiding this comment.
I don't think this change is making it more readable. I'd rather stick with the long if else than do string operations that is hard to read and understand what it actually does. Simple is better
|
|
||
| @Override | ||
| public boolean equals(Object obj) { | ||
| Skin skin = (Skin) obj; | ||
| return name.equals(skin.name); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return name.hashCode(); | ||
| } |
There was a problem hiding this comment.
It's been more than 4 years since I've last looked at this repository, but from looking at this method, I think I wanted to have Skin instances to be equal even if only name property matches. I assume Lombok generates a default equals and hash function using all the properties of the object (again, haven't used it for a long while now), which is not the same behavior with these overridden methods. Can you check?
There was a problem hiding this comment.
It's same. It's like that. Lombok does same thing.
There was a problem hiding this comment.
From https://projectlombok.org/features/EqualsAndHashCode:
By default, it'll use all non-static, non-transient fields, but you can modify which fields are used (and even specify that the output of various methods is to be used) by marking type members with
@EqualsAndHashCode.Includeor@EqualsAndHashCode.Exclude. Alternatively, you can specify exactly which fields or methods you wish to be used by marking them with@EqualsAndHashCode.Includeand using@EqualsAndHashCode(onlyExplicitlyIncluded = true)
|
Also, since this is not a maven/gradle project, can you add some instructions for how to install Lombok on Eclipse/IntelliJ/VSCode IDEs so people with less knowledge of java can also build the jar file? |
|
|
||
| @Override | ||
| public boolean equals(Object obj) { | ||
| Skin skin = (Skin) obj; | ||
| return name.equals(skin.name); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return name.hashCode(); | ||
| } |
There was a problem hiding this comment.
It's same. It's like that. Lombok does same thing.
|
|
||
| @Override | ||
| public boolean equals(Object obj) { | ||
| Skin skin = (Skin) obj; | ||
| return name.equals(skin.name); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return name.hashCode(); | ||
| } |
There was a problem hiding this comment.
From https://projectlombok.org/features/EqualsAndHashCode:
By default, it'll use all non-static, non-transient fields, but you can modify which fields are used (and even specify that the output of various methods is to be used) by marking type members with
@EqualsAndHashCode.Includeor@EqualsAndHashCode.Exclude. Alternatively, you can specify exactly which fields or methods you wish to be used by marking them with@EqualsAndHashCode.Includeand using@EqualsAndHashCode(onlyExplicitlyIncluded = true)
| public String getLang() { | ||
| if (name.contains("phase1")) | ||
| { | ||
| return lang + " (Phase 1)"; | ||
| } | ||
| else if (name.contains("phase2")) | ||
| { | ||
| return lang + " (Phase 2)"; | ||
| } | ||
| else if (name.contains("phase3")) | ||
| { | ||
| return lang + " (Phase 3)"; | ||
| } | ||
| else if (name.contains("phase4")) | ||
| { | ||
| return lang + " (Phase 4)"; | ||
| } | ||
| else if (name.contains("emerald_marbleized")) | ||
| { | ||
| return lang + " (Emerald)"; | ||
| } | ||
| else if (name.contains("ruby_marbleized")) | ||
| { | ||
| return lang + " (Ruby)"; | ||
| } | ||
| else if (name.contains("sapphire_marbleized")) | ||
| { | ||
| return lang + " (Sapphire)"; | ||
| } | ||
| else if (name.contains("blackpearl_marbleized")) | ||
| { | ||
| if (name.contains("phase")) { | ||
| return lang + " (" + capitalize(name.substring(0, 1)) + " " + name.charAt(5) + ")"; | ||
| } else if (name.contains("marbleized") && !name.contains("blackpearl")) { | ||
| return lang + " (" + capitalize(name.replace("marbleized", "")) + ")"; | ||
| } else if (name.contains("blackpearl_marbleized")) { | ||
| return lang + " (Black Pearl)"; | ||
| } | ||
| return lang; |
There was a problem hiding this comment.
As I said on my previous review, this looks worse than it was before. There's no need for hard to read string operations just to reduce the number of lines of code.
No description provided.