Pokemon type evaluator - follow-up

I started anew and completely refactored the way I implemented Pokemon type evaluator.

Also, first use of a lambda expression (Case of using it, but still don't fully grasp how it's working). I'm wondering if this is a bit more complex than it needs to be, but otherwise I think these are some significant improvements.

Element enum:

public enum Element {
    NONE("None"),
    NORMAL("Normal"),
    FIRE("Fire"),
    WATER("Water"),
    ELECTRIC("Electric"),
    GRASS("Grass"),
    BUG("Bug"),
    POISON("Poison"),
    ICE("Ice"),
    DRAGON("Dragon"),
    FAIRY("Fairy"),
    STEEL("Steel"),
    ROCK("Rock"),
    GROUND("Ground"),
    FIGHTING("Fighting"),
    FLYING("Flying"),
    PSYCHIC("Psychic"),
    DARK("Dark"),
    GHOST("Ghost");

    public final String name;

    Element(String name) {
        this.name = name;
    }
}

PokemonType class:

```
import java.util.HashMap;

public class PokemonType {
String name;
HashMap multipliers = new HashMap();
Element element;

PokemonType(Builder p) {
name = p.name; multipliers = p.multipliers; element = p.element;
}

static class Builder {
final String name;
HashMap multipliers = new HashMap();
Element element;

public Builder(Element element) {
this.name = element.name;
this.element = element;
// Default values
for (Element e : Element.values()) {
multipliers.put(e, 1.0);
}
}

public Builder vulnerableTo(Element... elements) {
for (Element e : elements) { multipliers.put(e, 2.0); }
return this;
}
public Builder immuneTo(Element... elements) {
for (Element e : elements) { multipliers.put(e, 0.0); }
return this;
}
public Builder resistantTo(Element... elements) {
for (Element e : elements) { multipliers.put(e, 0.5); }
return this;
}
pub

I think the overall design is good, although make the main method create a new PokemonTypeChecker and then move all the stuff currently in the main method to a run method or similar - doing logic in the constructor is bad practice, and not writing OO code (i.e. making everything static) is too. You could also perhaps consider JavaFX over swing/awt too.

Comments:

• It seems like Type may be a better name than Element?

• It's good practice to make everything as private as possible, so consider making the Element constructor private.

• Override toString in the enumerated type instead of having to specify a name in the constructor (you could utilise name).

-
It's good practice to make classes that shouldn't be extended final.

-
Use the diamond operator to make it quicker to change the left hand side of a generic class, and faster to write.

• Use the least-specific (but relevant) parent class on the LHS to make it quicker to change and, usually, write (as stated in Effective Java).

Old:

HashMap multipliers = new HashMap();

New:

Map multipliers = new HashMap<>();

• Is there a reason you're using JTextFields over a JLabel or similar? You're preventing them from being edited anyway.

• You might as well initialise empty StringBuilders instead of specifying the empty string literal in the constructor (they're identical).

• If the return none; part of the getPokemonType method should never happen, then you probably want to throw an unchecked exception there (likely IllegalArgumentException).

• Looks like the you could make use of Stream#filter in the displayAttributes.

Possibly subjective points I wanted to include regardless:

• Conventionally the method is called "build" rather than generate.

• I prefer Guava's way of instantiating builders because then you don't need to type the inner class (MyBuilder.Builder) and it's shorter.

StackExchange Code Review Q#62677, answer score: 8