3
\$\begingroup\$

I've just finished writing a program to help track Pokemon across Soul Links. The goal of the program is to take in the names of the two Pokemon alongside the route that they were caught on. The Pokemon is then added to a table for the user to keep track of whether or not the Pokemon has died.

import java.awt.Color;
import java.awt.Component;
import java.awt.Container;
import java.awt.Dimension;
import java.awt.Font;
import java.awt.FontMetrics;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.Insets;
import java.awt.RenderingHints;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;
import java.awt.event.WindowAdapter;
import java.awt.event.WindowEvent;
import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.Vector;

import javax.imageio.ImageIO;
import javax.swing.ImageIcon;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JScrollPane;
import javax.swing.JTable;
import javax.swing.JTextField;
import javax.swing.UIManager;
import javax.swing.UIManager.LookAndFeelInfo;
import javax.swing.UnsupportedLookAndFeelException;
import javax.swing.table.DefaultTableCellRenderer;
import javax.swing.table.DefaultTableModel;
import javax.swing.table.TableColumnModel;

public class PokeTracker extends JFrame {

    private static final long serialVersionUID = 1L;

    private static final int WIDTH = 600;
    private static final int HEIGHT = WIDTH / 2 + WIDTH / 6;
    private static final String TITLE = "PokéTracker";

    private String name1 = "Ash";
    private String name2 = "Brock";
    private int fontSize = 12;
    private int columns = 14;
    private int margin = fontSize / 2 + columns / 2;

    private List<Pokemon> pokemon;
    private DefaultTableModel model;

    public PokeTracker() throws IOException {
        this.pokemon = new ArrayList<Pokemon>();
        this.model = new DefaultTableModel() {

            private static final long serialVersionUID = 1L;

            Class<?>[] types = new Class<?>[] { String.class, String.class, String.class };
            boolean[] editable = new boolean[] { false, false, true };

            @Override
            public Class<?> getColumnClass(int columnIndex) {
                return this.types[columnIndex];
            }

            @Override
            public boolean isCellEditable(int rowIndex, int columnIndex) {
                return this.editable[columnIndex];
            }

        };

        JPanel panel = new JPanel(null);

        JLabel trainerLabel1 = new JLabel(name1 + "'s Pokémon");
        trainerLabel1.setBounds(margin, margin, 116, 30);
        panel.add(trainerLabel1);
        JTextField pokemonNameField1 = new HintTextField("name");
        pokemonNameField1.setBounds(trainerLabel1.getX() + trainerLabel1.getWidth(), margin, columns * 10, columns * 2);
        panel.add(pokemonNameField1);
        JLabel trainerLabel2 = new JLabel(name2 + "'s Pokémon");
        trainerLabel2.setBounds(margin, fontSize + margin * 2, 116, 30);
        panel.add(trainerLabel2);
        JTextField pokemonName2 = new HintTextField("name");
        pokemonName2.setBounds(trainerLabel2.getX() + trainerLabel2.getWidth(), fontSize + margin * 2, columns * 10, columns * 2);
        panel.add(pokemonName2);
        JTextField pokemonRoute = new HintTextField("route");
        pokemonRoute.setBounds(pokemonNameField1.getX() + pokemonNameField1.getWidth(), margin * 2, columns * 10, columns * 2);
        panel.add(pokemonRoute);

        JButton okButton = new JButton("OK");
        okButton.setBounds(pokemonRoute.getX() + pokemonRoute.getWidth() + margin / 4, pokemonRoute.getY() + pokemonRoute.getHeight() / 4, 48, pokemonRoute.getHeight() / 2);
        okButton.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                addToTable(new Pokemon(pokemonNameField1.getText().isBlank() ? Pokemon.NAMELESS : name1 + "'s " + pokemonNameField1.getText(), pokemonRoute.getText().isBlank() ? Pokemon.ROUTELESS : pokemonRoute.getText()));
                addToTable(new Pokemon(pokemonName2.getText().isBlank() ? Pokemon.NAMELESS : name2 + "'s " + pokemonName2.getText(), pokemonRoute.getText().isBlank() ? Pokemon.ROUTELESS : pokemonRoute.getText()));
                pokemonNameField1.setText("");
                pokemonName2.setText("");
                pokemonRoute.setText("");
            }
        });
        panel.add(okButton);

        JTable t = new JTable(model);

        t.setRowSelectionAllowed(true);
        t.setBounds(margin - 1, pokemonName2.getY() + pokemonName2.getHeight() + margin, WIDTH - (margin * 2), HEIGHT - 100 - margin);
        t.setDefaultRenderer(String.class, new CustomTableRenderer());
        t.addKeyListener(new KeyAdapter() {
            @Override
            public void keyPressed(KeyEvent e) {
                if (e.getKeyCode() == KeyEvent.VK_DELETE || e.getKeyCode() == KeyEvent.VK_BACK_SPACE) removeFromTable(t);
            }
        });

        model.addColumn("Name");
        model.addColumn("Route");
        model.addColumn("Dead");

        t.setRowHeight(16);
        TableColumnModel columnModel = t.getColumnModel();
        columnModel.getColumn(0).setPreferredWidth(200);
        columnModel.getColumn(1).setPreferredWidth(100);
        columnModel.getColumn(2).setPreferredWidth(5);

        load("data.csv");

        JScrollPane sp = new JScrollPane(t);
        sp.setBounds(t.getX(), t.getY(), t.getWidth(), t.getHeight());
        panel.add(sp);

        JLabel icon = new JLabel(new ImageIcon(ImageIO.read(new File("ico.png")).getScaledInstance(64, 64, Image.SCALE_SMOOTH)));
        int x = okButton.getX() + okButton.getWidth() + margin;
        int y = margin / 2 + 2;
        icon.setBounds(x, y, WIDTH - x - margin, HEIGHT - t.getHeight() - margin * 4 + 1);
        panel.add(icon);

        setAllFont(panel, new Font("Menlo", Font.PLAIN, fontSize));

        setDefaultCloseOperation(DO_NOTHING_ON_CLOSE);
        addWindowListener(new WindowAdapter() {
            @Override
            public void windowClosing(WindowEvent e) {
                save(t);
                System.exit(0);
            }
        });
        setTitle(TITLE);
        Dimension size = new Dimension(WIDTH, HEIGHT);
        getContentPane().setMinimumSize(size);
        getContentPane().setMaximumSize(size);
        getContentPane().setPreferredSize(size);
        getContentPane().add(panel);
        pack();
        setLocationByPlatform(true);
        setLocationRelativeTo(null);
        setResizable(false);
        setVisible(true);
    }

    public void load(String f) {
        try (BufferedReader br = Files.newBufferedReader(Paths.get(f), StandardCharsets.US_ASCII)) {
            String line = br.readLine();

            while (line != null) {
                String[] attributes = line.split(",");
                String name = attributes[0];
                String route = attributes[1];
                boolean dead = (attributes[2].contains("false") ? false : true);

                addToTable(new Pokemon(name.substring(6, name.length()), route.substring(7, route.length()), dead));

                line = br.readLine();
            }
        } catch (IOException e) {
            e.printStackTrace();
        }

    }

    public void save(JTable t) {
        if (!isEmpty(t)) {
            Vector<?> row = (Vector<?>) model.getDataVector().elementAt(1);

            for (int i = 0; i < t.getRowCount(); i++) {
                row = (Vector<?>) model.getDataVector().elementAt(i);
                Pokemon p = pokemon.get(i);
                /** synchronizing list of pokemon with table representation */
                p.shoudlDie((row.get(2).toString().toLowerCase().equals("false") ? false : true));
            }
        }

        try (BufferedWriter writer = new BufferedWriter(new FileWriter("data.csv"))) {
            pokemon.forEach(pokemon -> {
                try {
                    writer.append(pokemon.toString() + "\n");
                } catch (IOException e) {
                    e.printStackTrace();
                }
            });
        } catch (IOException e) {
            e.printStackTrace();
        }

    }

    void addToTable(Pokemon p) {
        model.addRow(new String[] { p.getName(), p.getRoute(), (p.isDead()) ? "true" : "false" });
        pokemon.add(p);
    }

    public static boolean isEmpty(JTable t) {
        if (t != null && t.getModel() != null) return t.getModel().getRowCount() <= 0 ? true : false;
        return false;
    }

    public void removeFromTable(JTable t) {
        int[] rows = t.getSelectedRows();
        if (t.getSelectedRow() >= 0) pokemon.remove(t.getSelectedRow());
        for (int i = 0; i < rows.length; i++) model.removeRow(rows[i] - i);
    }

    public void setAllFont(Component c, Font f) {
        c.setFont(f);
        if (c instanceof Container) for (Component child : ((Container) c).getComponents()) {
            setAllFont(child, (child instanceof JButton) ? new Font("Menlo", Font.PLAIN, 11) : f);
        }
    }

    public static void main(String[] args) throws IOException {
        try {
            for (LookAndFeelInfo info : UIManager.getInstalledLookAndFeels()) if ("Metal".equals(info.getName())) {
                UIManager.setLookAndFeel(info.getClassName());
                break;
            }
        } catch (Exception e) {
            try {
                UIManager.setLookAndFeel(UIManager.getCrossPlatformLookAndFeelClassName());
            } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex) {
                ex.printStackTrace();
            }
        }
        new PokeTracker();
    }

    class CustomTableRenderer extends DefaultTableCellRenderer {

        private static final long serialVersionUID = 1L;

        @Override
        public Component getTableCellRendererComponent(JTable t, Object value, boolean isSelected, boolean hasFocus, int row, int column) {
            Component component = super.getTableCellRendererComponent(t, value, isSelected, hasFocus, row, column);
            String data = t.getValueAt(row, 0).toString();

            if (data.contains(name1)) component.setForeground(Color.RED);
            else if (data.contains(name2)) component.setForeground(Color.BLUE);
            else component.setForeground(Color.BLACK);
            return component;
        }

    }

    /** https://stackoverflow.com/a/24571681 */
    class HintTextField extends JTextField {

        private static final long serialVersionUID = 1L;

        public HintTextField(String hint) {
            _hint = hint;
        }

        @Override
        public void paint(Graphics g) {
            super.paint(g);
            if (getText().length() == 0) {
                int h = getHeight();
                ((Graphics2D) g).setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING, RenderingHints.VALUE_TEXT_ANTIALIAS_ON);
                Insets ins = getInsets();
                FontMetrics fm = g.getFontMetrics();
                int c0 = getBackground().getRGB();
                int c1 = getForeground().getRGB();
                int m = 0xfefefefe;
                int c2 = ((c0 & m) >>> 1) + ((c1 & m) >>> 1);
                g.setColor(new Color(c2, true));
                g.drawString(_hint, ins.left, h / 2 + fm.getAscent() / 2 - 2);
            }
        }

        private final String _hint;
    }

    class Pokemon {

        public static final String NAMELESS = "???";
        public static final String ROUTELESS = "???";

        private String name;
        private String route;
        private boolean dead;

        public Pokemon(String name, String route, boolean dead) {
            this.name = name;
            this.route = route;
            this.dead = dead;
        }

        public Pokemon(String name, String route) {
            this.name = name;
            this.route = route;
            this.dead = false;
        }

        public Pokemon(String name) {
            this.name = name;
            this.route = ROUTELESS;
            this.dead = false;

        }

        public Pokemon() {
            this.name = NAMELESS;
            this.route = ROUTELESS;
            this.dead = false;
        }

        public String getName() {
            return name;
        }

        public void setName(String name) {
            this.name = name;
        }

        public String getRoute() {
            return route;
        }

        public void setRoute(String route) {
            this.route = route;
        }

        public boolean isDead() {
            return dead;
        }

        public void shoudlDie(boolean dead) {
            this.dead = dead;
        }
        
        @Override
        public String toString() {
            return "name: " + name + ",route: " + route + ",dead: " + dead + ",";
        }

    }

}
\$\endgroup\$
2
  • \$\begingroup\$ So, what is it that you want a reviewer to do with your code? \$\endgroup\$ – Gilbert Le Blanc Mar 5 at 5:20
  • \$\begingroup\$ I guess tell me any malpractices or flaws in my code. I’m self taught so I’m not 100% sure that anything I’m doing is the “correct” or best way to do it. \$\endgroup\$ – Temps Mar 5 at 19:01
3
\$\begingroup\$

There are too many input statements. This is a red flag; it usually means that the classes try and do to much.


private String name1 = "Ash";

First of all, when we start counting, we should stop and use a list. Second, you're mixing in the program logic with the GUI here.


private int columns = 14;
private int margin = fontSize / 2 + columns / 2;

This probably means that we're talking about a columnSize here, not an amount of columns.


Class<?>[] types = new Class<?>[] { String.class, String.class, String.class };
boolean[] editable = new boolean[] { false, false, true };

This is overly verbose, even for Java:

Class<?>[] types = { String.class, String.class, String.class };
boolean[] editable = { false, false, true };

JLabel trainerLabel1 = new JLabel(name1 + "'s Pokémon");

Those identifiers such as trainerLabel1 can do without the 1 and 2 whatnot counting. Names should make sense by themselves, the 1 and 2 that are often automatically added should be removed or replaced by something more descriptive.


trainerLabel1.setBounds(margin, margin, 116, 30);

If possibly, try to design a GUI in such a way that explicit pixels / point counts are used as little as possible. This helps e.g. with scaling. In other words, please do use a LayoutManager, not null.

okButton.setBounds(pokemonRoute.getX() + pokemonRoute.getWidth() + margin / 4, pokemonRoute.getY() + pokemonRoute.getHeight() / 4, 48, pokemonRoute.getHeight() / 2);

Unfortunately the "why" behind all the calculations is not made clear. This would require a comment to help future developers (such as yourself, trust me on that one).


addToTable(new Pokemon(pokemonNameField1.getText().isBlank() ? Pokemon.NAMELESS : name1 + "'s " + pokemonNameField1.getText(), pokemonRoute.getText().isBlank() ? Pokemon.ROUTELESS : pokemonRoute.getText()));
addToTable(new Pokemon(pokemonName2.getText().isBlank() ? Pokemon.NAMELESS : name2 + "'s " + pokemonName2.getText(), pokemonRoute.getText().isBlank() ? Pokemon.ROUTELESS : pokemonRoute.getText()));

These lines are overly complex and repetitive. That usually means that you have to refactor something into a function (or two).


JTable t = new JTable(model);

We're a bit into the class, and we've given up on descriptive names entirely. That's a shame, because table and pokeTrackerTable was still free.


load("data.csv");

Glad you used a method here. But it would be even nicer if you did not use hard coded file names, and if you do, at least make them a constant somewhere on the top of the code.


JScrollPane sp = new JScrollPane(t);
sp.setBounds(t.getX(), t.getY(), t.getWidth(), t.getHeight());
panel.add(sp);

Even small fractions of code like these can be refactored into a method (addScrollPane) which unclutters the code.


    JLabel icon = new JLabel(new ImageIcon(ImageIO.read(new File("ico.png")).getScaledInstance(64, 64, Image.SCALE_SMOOTH)));

Again, you're doing too much in this line. Never nest IO operations. They are common places of failure, and you don't want to clutter the debugging part. Also, icons are generally created from resources, not from local files.


setTitle(TITLE);

Hmm, this feels a bit too much like an afterthought to me; what about:

super(TITLE);

as the very first line of code for this method?


boolean dead = (attributes[2].contains("false") ? false : true);

Oh, really? "not false" is false and "False" is true? Sometimes more strict definitions help a lot keeping things in line.


addToTable(new Pokemon(name.substring(6, name.length()), route.substring(7, route.length()), dead));

Now you're not just mixing program logic with the GUI, but I/O operations as well. Please keep these all separate. Load to a table in memory and display that.

Furthermore, a bit of magic happens here: a substring from location 7 (what's that?) from an otherwise unvalidated string. That's just waiting for errors to happen.


e.printStackTrace();

No, no, no. You never keep running on an error. NEVER leave the printing of the stack trace in. If you need to handle errors later:

// TODO handle exception
throw new RuntimeException("Unhandled exception", e);

This will still print out the stacktrace...

I have no clue why popular IDE's don't do this by default, but it sucks badly that they don't.


Vector<?> row = (Vector<?>) model.getDataVector().elementAt(1);

If you keep using values like 1, 2 and 3 to mean something, you might need either constants or - even better - enumerations. And if those get too complex, you probably need full featured (data) classes.


p.shoudlDie((row.get(2).toString().toLowerCase().equals("false") ? false : true));

Question: what does equals return?


void addToTable(Pokemon p) {

In general there needs to be a good reason for the access modifier to be absent (package-private). Generally, methods such as these should be private as you would not expect them to be called by other classes (this is easier if the class just focuses on managing the GUI).


public static boolean isEmpty(JTable t) {

I got this far when I wondered where the table was stored. Finding out that it is simply a variable local to the constructor was a bit of a surprise. The good thing is that you don't need all the null checking in that case. Of course, that would mean making the method private.


    if (t.getSelectedRow() >= 0) pokemon.remove(t.getSelectedRow());

That's weird, the pokemon is a field and the table isn't? I'd expect remove(Pokemon pokemon) to be a lot more intuitive.


public void setAllFont(Component c, Font f) {

Here you show that you are definitely capable of splitting up smaller methods, great.

for (LookAndFeelInfo info : UIManager.getInstalledLookAndFeels()) if ("Metal".equals(info.getName())) {

No, please use parentheses for if statements and put them on a separate line. I don't think it is a problem that info will ever be null or that it has a null name, so the rather stupid "literal".equals(value) construct is unnecessary.

Generally, just use value.equals("literal") please, it makes for a much better reading experience. In the few cases that value can be null, handle them separately.


ex.printStackTrace();

Why? Just do nothing, otherwise this will remain and the user will get an unnecessary trace to handle (if setting the L&F fails).


        if (data.contains(name1)) component.setForeground(Color.RED);

contains is really unnecessary here. If it is just a name then you could also use a switch with a String nowadays.

However, what about:

public enum Pokemon {
    ASH("Ash", Color.RED),
    BROCK("Brock", Color.BLUE),
    UNKNOWN("???", Color.BLACK);
}

I'll leave creation of the fiels, private contructor and getters up to you.


 /** https://stackoverflow.com/a/24571681 */

This is great, because links to previos documents are really necessary. However, I'd not put it in a JavaDoc without a further description (but I hope other comments are just stripped for this review).

The class could be private though, or separated out because it is rather large. It probably should be static as I don't see it using any fields of the main class.


 _hint = hint;

etc. Copying is fine, but if the code uses bad identifiers, then please correct them before you get swamped by e.g. a run of CheckStyle.


 public void shoudlDie(boolean dead) {

Your Pokemon can actually be immutable, if not for the state. I'd use another method to keep the state, maybe a table or something. Because now you can resurrect Pokemon and have zombie-pokemons. That's called an "invalid state" and those should always be avoided.

Actually, it seems like this is explicitly stated as game rule:

If a pokemon dies, you cannot revive it.

\$\endgroup\$
1
  • \$\begingroup\$ A little hint: I often first create a textual interface for my game. That requires me to program the game logic separately from the GUI code. Then I can add the GUI code later. Of course, if I use console output in my game logic I still fail. \$\endgroup\$ – Maarten Bodewes Mar 6 at 12:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.