I thought I would apply what I learned to make something fun. Its purpose is to type check Pokemon, for anyone familiar with the series. It mostly works but I have two concerns:Sometimes you write code and you get the pervading feeling you're not using some method, some API or the best approach available; this is one of those times for me. It definitely doesn't seem like an if this is the case. Where could it be improved upon?For comments, the last time I posted one of these I was told I should try to write code that is self-evident and doesn't need commenting.  I tried that this time, but I feel I'd rather have possibly redundant comments than difficult to follow code that diminishes review quality. I have a commented version but I thought I'd first try the plain one (excepting one to explain a personal habit). What is the general consensus on the best approach?import java.awt.BorderLayout;import java.awt.event.ItemEvent;import java.awt.event.ItemListener;import java.awt.GridLayout;import javax.swing.ImageIcon;import javax.swing.JComboBox;import javax.swing.JFrame;import javax.swing.JLabel;import javax.swing.JPanel;import javax.swing.JTextField;import javax.swing.WindowConstants;public class TypeChecker {    static Double normMulti1 = 1.0, flyMulti1 = 1.0, fightMulti1 = 1.0, fireMulti1 = 1.0,        waterMulti1 = 1.0, elecMulti1 = 1.0, grassMulti1 = 1.0, bugMulti1 = 1.0,        poisMulti1 = 1.0, darkMulti1 = 1.0, psyMulti1 = 1.0, ghostMulti1 = 1.0,        groundMulti1 = 1.0, rockMulti1 = 1.0, steelMulti1 = 1.0, iceMulti1 = 1.0,        dragMulti1 = 1.0, faeMulti1 = 1.0;    static String[] elements = {Normal, Flying, Fighting, Fire, Water,        Electric, Grass, Bug, Poison, Dark, Psychic, Ghost,        Ground, Rock, Steel, Ice, Dragon, Fairy};    static int len = elements.length;    static Double[] multiplierList1 = new Double[len];    static JTextField immune = new JTextField(15), res = new JTextField(15),        vuln = new JTextField(15);    public static void main(String[] args) {        JFrame frame = new JFrame(Type Checker);        JPanel panel = new JPanel(), result = new JPanel();        result.setLayout(new GridLayout(3, 2, 0, 2));        result.add(new JLabel(Immune to:));        result.add(immune);        result.add(new JLabel(Resistant to:));        result.add(res);        result.add(new JLabel(Vulnerable to:));        result.add(vuln);        vuln.setEditable(false);        res.setEditable(false);        immune.setEditable(false);        multiplierSet();        ImageIcon[] types = new ImageIcon[len];        for (int i = 0; i < len; i++) {            types[i] = new ImageIcon(Images/ + elements[i] + .gif);        }        JComboBox<ImageIcon> typeList = new JComboBox<ImageIcon>(types);        typeList.addItemListener(new ItemListener() {            @Override            public void itemStateChanged(ItemEvent e) {                if (e.getStateChange() == ItemEvent.SELECTED) {                    multiplierReset();                    multiplierDisplay();                    for (int i = 0; i < len; i++) {                        if (e.getItem() == types[i]) {                            switch(elements[i]) {                                case Normal:                                    fightMulti1 = 2.0;                                    ghostMulti1 = 0.0;                                    break;                                case Flying:                                    elecMulti1 = 2.0; iceMulti1 = 2.0; rockMulti1 = 2.0;                                    bugMulti1 = 0.5; fightMulti1 = 0.5; grassMulti1 = 0.5;                                    groundMulti1 = 0.0;                                    break;                                case Fighting:                                    flyMulti1 = 2.0; psyMulti1 = 2.0;                                    bugMulti1 = 0.5; rockMulti1 = 0.5;                                    break;                                case Fire:                                    groundMulti1 = 2.0; rockMulti1 = 2.0; waterMulti1 = 2.0;                                    bugMulti1 = 0.5; faeMulti1 = 0.5; fireMulti1 = 0.5;                                    grassMulti1 = 0.5; iceMulti1 = 0.5; steelMulti1 = 0.5;                                    break;                                case Water:                                    elecMulti1 = 2.0; grassMulti1 = 2.0;                                    fireMulti1 = 0.5; iceMulti1 = 0.5;                                    steelMulti1 = 0.5; waterMulti1 = 0.5;                                    break;                                case Electric:                                    groundMulti1 = 2.0;                                    elecMulti1 = 0.5; flyMulti1 = 0.5; steelMulti1 = 0.5;                                    break;                                case Grass:                                    bugMulti1 = 2.0; iceMulti1 = 2.0;                                    fireMulti1 = 2.0; flyMulti1 = 2.0; poisMulti1 = 2.0;                                    elecMulti1 = 0.5; grassMulti1 = 0.5;                                    groundMulti1 = 0.5; waterMulti1 = 0.5;                                    break;                                case Bug:                                    fireMulti1 = 2.0; flyMulti1 = 2.0; rockMulti1 = 2.0;                                    fightMulti1 = 0.5; grassMulti1 = 0.5; groundMulti1 = 0.5;                                    break;                                case Poison:                                    groundMulti1 = 2.0; psyMulti1 = 2.0;                                    bugMulti1 = 0.5; faeMulti1 = 0.5;                                    fightMulti1 = 0.5; grassMulti1 = 0.5; poisMulti1 = 0.5;                                    break;                                case Dark:                                    bugMulti1 = 2.0; faeMulti1 = 2.0; fightMulti1 = 2.0;                                    darkMulti1 = 0.5; ghostMulti1 = 0.5;                                    psyMulti1 = 0.0;                                    break;                                case Psychic:                                    bugMulti1 = 2.0; darkMulti1 = 2.0; ghostMulti1 = 2.0;                                    fightMulti1 = 0.5; psyMulti1 = 0.5;                                    break;                                case Ghost:                                    darkMulti1 = 2.0; ghostMulti1 = 2.0;                                    bugMulti1 = 0.5; poisMulti1 = 0.5;                                    fightMulti1 = 0.0; normMulti1 = 0.0;                                    break;                                case Ground:                                    iceMulti1 = 2.0; grassMulti1 = 2.0; waterMulti1 = 2.0;                                    poisMulti1 = 0.5; rockMulti1 = 0.5;                                    elecMulti1 = 0.0;                                    break;                                case Rock:                                    fightMulti1 = 2.0; grassMulti1 = 2.0;                                    groundMulti1 = 2.0; steelMulti1 = 2.0; waterMulti1 = 2.0;                                    fireMulti1 = 0.5; flyMulti1 = 0.5;                                    normMulti1 = 0.5; poisMulti1 = 0.5;                                    break;                                case Steel:                                    fightMulti1 = 2.0; fireMulti1 = 2.0; groundMulti1 = 2.0;                                    bugMulti1 = 0.5; dragMulti1 = 0.5; faeMulti1 = 0.5;                                    flyMulti1 = 0.5; grassMulti1 = 0.5; iceMulti1 = 0.5;                                    normMulti1 = 0.5; psyMulti1 = 0.5;                                    rockMulti1 = 0.5; steelMulti1 = 0.5;                                    poisMulti1 = 0.0;                                    break;                                case Ice:                                    fightMulti1 = 2.0; fireMulti1 = 2.0;                                    rockMulti1 = 2.0; steelMulti1 = 2.0;                                    iceMulti1 = 0.5;                                    break;                                case Dragon:                                    dragMulti1 = 2.0; faeMulti1 = 2.0; iceMulti1 = 2.0;                                    elecMulti1 = 0.5; fireMulti1 = 0.5;                                    grassMulti1 = 0.5; waterMulti1 = 0.5;                                    break;                                case Fairy:                                    poisMulti1 = 2.0; steelMulti1 = 2.0;                                    bugMulti1 = 0.5; darkMulti1 = 0.5; fightMulti1 = 0.5;                                    dragMulti1 = 0.0;                                break; // final break purposely indented such                            }                            multiplerSet();                            multiplierDisplay();                        }                    }                }            }        });        panel.add(typeList);        frame.setIconImage(new ImageIcon(Images/Icon.png).getImage());        frame.add(result, BorderLayout.CENTER);        frame.add(panel, BorderLayout.SOUTH);        frame.pack();        frame.setLocationRelativeTo(null);        frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);        frame.setVisible(true);    }    public static void multiplierSet() {        multiplierList1[0] = normMulti1;        multiplierList1[1] = flyMulti1;        multiplierList1[2] = fightMulti1;        multiplierList1[3] = fireMulti1;        multiplierList1[4] = waterMulti1;        multiplierList1[5] = elecMulti1;        multiplierList1[6] = grassMulti1;        multiplierList1[7] = bugMulti1;        multiplierList1[8] = poisMulti1;        multiplierList1[9] = darkMulti1;        multiplierList1[10] = psyMulti1;        multiplierList1[11] = ghostMulti1;        multiplierList1[12] = groundMulti1;        multiplierList1[13] = rockMulti1;        multiplierList1[14] = steelMulti1;        multiplierList1[15] = iceMulti1;        multiplierList1[16] = dragMulti1;        multiplierList1[17] = faeMulti1;    }    public static void multiplierReset() {        for (int i = 0; i < len; i++) {            multiplierList1[i] = 1.0;        }    }    public static void multiplierDisplay() {        StringBuilder vln = new StringBuilder(), resist = new StringBuilder(),            imm = new StringBuilder();;        for (int i = 0; i < len; i++) {            if (multiplierList1[i] == 0.0) {                if (imm.toString().isEmpty()) { imm.append(elements[i]); }                else { imm.append(,  + elements[i]); }            }            else if (multiplierList1[i] == 0.5) {                if (resist.toString().isEmpty()) { resist.append(elements[i]); }                else { resist.append(,  + elements[i]); }            }            else if (multiplierList1[i] == 2) {                if (vln.toString().isEmpty()) { vln.append(elements[i]); }                else { vln.append(,  + elements[i]); }            }        }        if (vln.toString().isEmpty()) { vln.append(None); }        if (imm.toString().isEmpty()) { imm.append(None); }        if (resist.toString().isEmpty()) { resist.append(None); }        immune.setText(imm.toString());        res.setText(resist.toString());        vuln.setText(vln.toString());    }}
Pokemon type evaluator
java;beginner;swing;pokemon
Will provide a fuller review if I can later, but anyways here are some quick starters...See major edit for point 3.Encapsulate the generation of typeList JComboBox as a method so that contents can be easily swappedI definitely don't have access to your Images/ + elements[i] + .gif, and I don't expect them to be posted here. :D Therefore, in my humble opinion, I think you can put this generation in its own method so that you can easily change the contents without affecting the overall code. For example, in my own testing of your code, I have the following line:JComboBox<?> typeList = getComboBox();And then my implementation goes like the following, to use plain Strings:private static JComboBox<?> getComboBox() {    return new JComboBox<String>( elements );}switch on the selected item for clarityIn your current code, you do a for-loop and compare that the selected item is the same object by reference with a very large if-scope (==, which is usually not recommended unless you are really sure of your object referencing) before switching on your 'original' elements array. This sort-of makes sense still in your simpler code example, but it can get messy easily when you scale up for bigger projects as you will have a lot more references to use and keep track. It is therefore easier and clearer to perform the switch on the selected item directly, such that you can even remove the encompassing for and if conditions. For example, I can reduce this:for (int i = 0; i < len; i++) {    if (e.getItem() == types[i]) {        switch(elements[i]) {To just switch( e.getItem().toString() ). This is related to the previous point too, since I do not need to rely on your types ImageIcon array.Use enums to represent the various typesA great deal of your 'calculations' and repeated multiplerSet()/multiplerReset()/multiplerDisplay() method calls can be easily avoided with the right implementation of an enum, e.g. Pokemon. public enum Pokemon {    BUG, DARK, DRAGON...;    public static final Map<Pokemon, String> POKEMON_MAP = new EnumMap<>(Pokemon.class);    private final Set<Pokemon> immuneTo = new HashSet<>();    private final Set<Pokemon> resistantTo = new HashSet<>();    private final Set<Pokemon> vulnerableTo = new HashSet<>();}Since the Java naming convention for enum values is ALLCAPS and we want to display a more readable value on the GUI, we can use an EnumMap to store these representations. The three Sets is used to store the appropriate Pokemon types, and we will need to create getter and setter methods for them (using immuneTo as an example):public enum Pokemon {    // ... refer to above code    public final Set<Pokemon> getImmuneTo() {        return Collections.unmodifiableSet(immuneTo);    }    private Pokemon setImmuneTo(Pokemon... immuneTo) {        for (final Pokemon pokemon : immuneTo) {            add(pokemon, this.immuneTo);        }        return this;    }    private void add(final Pokemon type, final Set<Pokemon> set) {        if (!set.add(type)) {            throw new IllegalArgumentException(Duplicate addition.);        }    }}getImmuneTo is a public method that returns only an unmodifiable view of the underlying Set to prevent accidental changes, which is a good practice especially in this case. Conversely, setImmuneTo is a private method which will be used in the static block below to add Pokemon values at initialization. It conveniently returns itself for method chaining. Finally, the add() method is just used to defend against accidental repetition of values. :) The static block to initialize both the POKEMON_MAP and the values can be done in the following manner:public enum Pokemon {    // ... refer to above code    private static void initNameMap() {        for (final Pokemon pokemon : Pokemon.values()) {            POKEMON_MAP.put(pokemon, getFormattedName(pokemon));        }    }    private static String getFormattedName(final Pokemon pokemon) {        final String stringName = pokemon.toString();        final StringBuilder result = new StringBuilder(stringName.substring(0, 1));        return result.append(stringName.substring(1).toLowerCase()).toString();    }    static {        initNameMap();        BUG.setResistantTo(FIGHTING, GRASS, GROUND).setVulnerableTo(FIRE, FLYING, ROCK);        DARK.setImmuneTo(PSYCHIC).setResistantTo(DARK, GHOST).setVulnerableTo(BUG, FAIRY, FIGHTING);        DRAGON.setResistantTo(ELECTRIC, FIRE, GRASS, WATER).setVulnerableTo(DRAGON, FAIRY, ICE);        // ...    }}getFormattedName() simply 'converts' the default toString() value for, e.g. BUG, to Bug. As mentioned above, method chaining gives us a more fluent approach to specify the types of Pokemon that one is immune, resistant or vulnerable to.getComboBox() in your main class can now be simplified as such:private static JComboBox<?> getComboBox() {    return new JComboBox<>( Pokemon.POKEMON_MAP.values().toArray(new String[0]));}Inside your itemStateChanged() method, the code then becomes more readable (some small changes to your JTextField variable names):public void itemStateChanged(ItemEvent e) {    if (e.getStateChange() != ItemEvent.SELECTED) {        return;    }    for (final Entry<Pokemon, String> entry : Pokemon.POKEMON_MAP.entrySet()) {        if (entry.getValue().equals(e.getItem())) {            final Pokemon selected = entry.getKey();            immuneTo.setText(deriveResults(selected.getImmuneTo()));            resistantTo.setText(deriveResults(selected.getResistantTo()));            vulnerableTo.setText(deriveResults(selected.getVulnerableTo()));            break;        }    }}deriveResults() is just a method to convert your Set of Pokemon types to a String, and one possible approach is illustrated below:private static String deriveResults(final Set<Pokemon> pokemonSet) {    if (pokemonSet.isEmpty()) {        return None;    }    StringBuilder results = new StringBuilder();    for (final Pokemon pokemon : pokemonSet) {        if (results.length() > 0) {            results.append(, );        }        results.append(Pokemon.POKEMON_MAP.get(pokemon));    }    return results.toString();}Better use of StringBuilderSimply put, vln.append( ,  + elements[i] )should be re-written as vln.append( ,  ).append( elements[i] )If you are appending a single character, do something like builder.append( 'a' ) instead of builder.append( a ). If you want to use StringBuilder, use it efficiently.Better use of switchbreak; // final break purposely indented suchThis... isn't quite preferred. Normal convention dictates something like this, again for clarity:case <n>:    ...    break;default:    break;Use a better class nameTypeChecker can imply lots of types checking. You can easily rename it as PokemonTypeChecker. :)Consider a CustomRenderer for your JComboBox?I am a little unsure of this suggestion (I have to do some reading for myself as well), but maybe you can consider a CustomRenderer for your JComboBox?