Classic two-player memory game

Question

This is a classic memory game with a points counter for the two players.

The app works fine, but since this is my first project in Swing, I would appreciate the critical opinion of some expert, as I'm sure there is plenty of space for code improvement/optimization.

What do you think about the code? What should have I done in a different/better way? What are your recommendations in terms of optimization?

public class Pixeso extends JFrame {

    private JPanel contentPane;
    ImageIcon[] iconarray;
    ArrayList<Integer> list1 = new ArrayList<Integer>();
    JButton[] buttonarray = new JButton[20];
    Random rand1 = new Random();
    JButton button1;
    JButton button2;
    int counter = 1;
    Timer timer1;
    int points1;
    int points2;
    boolean player1 = true;
    private final JPanel panel1 = new JPanel();
    private final JPanel panel2 = new JPanel();
    private final JLabel label1 = new JLabel("0");
    private final JLabel label2 = new JLabel("0");
    private final JLabel lblNewLabel = new JLabel("Player 1");
    private final JLabel lblNewLabel_1 = new JLabel("Player 2");

    /**
     * Launch the application.
     */
    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                try {
                    Pixeso frame = new Pixeso();
                    frame.setVisible(true);
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }
        });
    }

    /**
     * Create the frame.
     * 
     * @throws IOException
     */
    public Pixeso() throws IOException {
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setBounds(100, 100, 1204, 908);
        contentPane = new JPanel();
        contentPane.setBackground(Color.GREEN);
        contentPane.setBorder(new EmptyBorder(0, 0, 0, 0));
        setContentPane(contentPane);
        contentPane.setLayout(new FlowLayout(FlowLayout.CENTER, 5, 5));
        panel1.setBackground(Color.GREEN);

        contentPane.add(panel1);
        panel1.setLayout(new GridLayout(5, 4, 5, 5));
        panel2.setBorder(new LineBorder(new Color(0, 0, 0)));

        contentPane.add(panel2);
        lblNewLabel.setHorizontalAlignment(SwingConstants.CENTER);
        lblNewLabel.setVerticalAlignment(SwingConstants.TOP);
        lblNewLabel.setFont(new Font("Tahoma", Font.PLAIN, 16));
        lblNewLabel.setBackground(Color.WHITE);
        label1.setHorizontalAlignment(SwingConstants.CENTER);
        label1.setVerticalAlignment(SwingConstants.TOP);
        label1.setForeground(Color.RED);
        label1.setFont(new Font("Tahoma", Font.PLAIN, 16));
        label1.setBackground(Color.WHITE);
        label2.setHorizontalAlignment(SwingConstants.CENTER);
        label2.setForeground(Color.RED);
        label2.setVerticalAlignment(SwingConstants.TOP);
        label2.setFont(new Font("Tahoma", Font.PLAIN, 16));
        label2.setBackground(Color.WHITE);
        panel2.setLayout(new GridLayout(2, 2, 20, 10));
        panel2.add(lblNewLabel);
        panel2.add(label1);
        lblNewLabel_1.setHorizontalAlignment(SwingConstants.CENTER);
        lblNewLabel_1.setVerticalAlignment(SwingConstants.TOP);
        lblNewLabel_1.setFont(new Font("Tahoma", Font.PLAIN, 16));
        lblNewLabel_1.setBackground(Color.WHITE);
        panel2.add(lblNewLabel_1);
        panel2.add(label2);

        // put the imagines in a URL[]
        URL[] immagini = new URL [11];
    immagini[0] = new URL("http://i.imgur.com/421DcmK.jpg");
    immagini[1] = new URL("http://i.imgur.com/mpx0yXN.jpg");
    immagini[2] = new URL("http://i.imgur.com/9i8UkrI.jpg");
    immagini[3] = new URL("http://i.imgur.com/KN86BKv.jpg");
    immagini[4] = new URL("http://i.imgur.com/KN86BKv.jpg");
    immagini[5] = new URL("http://i.imgur.com/mS3dRj7.jpg");
    immagini[10] = new URL("http://i.imgur.com/7vdVgHa.jpg");
    immagini[7] = new URL("http://i.imgur.com/njAuT7Q.jpg");
    immagini[8] = new URL("http://i.imgur.com/5hWZQG8.jpg");
    immagini[9] = new URL("http://i.imgur.com/bwZAiyL.jpg");
    immagini[6] = new URL("http://i.imgur.com/rHbAnOD.jpg");

        iconarray = new ImageIcon[11];
        // convert imagines in icons
        for (int i = 0; i <= 10; i++) {
            iconarray[i] = new ImageIcon(immagini[i]);
            // inizializzo list1
            list1.add(0);
        }

        // add 20 JButtons to panel 1 and set initial icon
        for (int i = 0; i < 20; i++) {
            panel1.add(new JButton(iconarray[10]));
            // insert JButtons in buttonarray
            buttonarray[i] = (JButton) panel1.getComponent(i);
            // add ImageButtonListener method to each JButton
            buttonarray[i].addActionListener(new ImageButtonListener());

        }

        // add a number between 0 and 9 for each JButton
        int y = 0;
        while (y < 20) {
            int x = rand1.nextInt(10);
            list1.set(x, list1.get(x).intValue() + 1);
            if (list1.get(x) <= 2) {
                buttonarray[y].setName(Integer.toString(x));
                y++;
            }
        }

        timer1 = new Timer(2000, new TimerListener());

    }

    // this timer show clicked cards for two seconds
    private class TimerListener implements ActionListener {
        public void actionPerformed(ActionEvent e) {
            button1.setIcon(iconarray[10]);
            button2.setIcon(iconarray[10]);
            timer1.stop();
            // active = true;
        }
    }

    // method to change JButton image
    class ImageButtonListener implements ActionListener {
        public void actionPerformed(ActionEvent e) {
            // waiting for timer to pop, user clicks not accepted
            if (timer1.isRunning())
                return;

            for (int i = 0; i < 20; i++)
                if (e.getSource() == buttonarray[i]) {
                    int x = Integer.parseInt(buttonarray[i].getName());
                    buttonarray[i].setIcon(iconarray[x]);

                    // button1=  first clicked button
                    if (counter == 1) {
                        button1 = buttonarray[i];
                        counter++;
                    }
                    // button 2= second clicked button, check I didn't click same card twice
                if (counter == 2 && buttonarray[i] != button1) {
                        button2 = buttonarray[i];
                        compareicons();
                    }
                }
        }

        // check if icons match
        private void compareicons() {
            if (button1.getIcon() == button2.getIcon()) {
                button1.setEnabled(false);
                button2.setEnabled(false);

        //add up points to player who found two matching icons
                if (player1 == true) {
                    points1++;
                    label1.setText(Integer.toString(points1));
                } else {
                    points2++;
                    label2.setText(Integer.toString(points2));
                }
            }

        //if cards are different, switch to other player
            else {
                if (player1 == true) {
                    player1 = false;
                } else {
                    player1 = true;
                }
                timer1.start();
            }
            //reset counter
            counter = 1;
        }
    }

}

Answer 1

I would have some suggestions on the code

• Make your fields private.
• Do not specify the implementation class, as in ArrayList<Integer> list1 = new ArrayList<Integer>();. Use List<Integer> list1 = new ArrayList<Integer>(); instead.
• Either initialize the fields in declaration or in constructor, don't mix both approach. I personally would prefer consistency in code.
• Give more meaningful names to variables, especially fields. Say what is button1 and button2? points1 and points2?
• In general use Lists instead of arrays directly even if it is fixed size. It's pretty legacy. In substitute I would use List<JButton> buttons = Arrays.asList(new JButton[20]);, and speaking of which
• Avoid hard-coding the number of things as literal. Especially in for-loop. Trouble ensues when the length changes. Use a constant like private static final int NUM_OF_BUTTONS = 20; and initialize the buttons as List<JButton> buttons = Arrays.asList(new JButton[NUM_OF_BUTTONS]);. Then for looping all the buttons, say for(int i = 0; i < buttons.size(); i++), or even better for(JButton button : buttons). In general, I would do that whenever I see I use the same literal twice.
• Avoid superfluous boolean equality checking or assignment. E.g. if (player1 == true) can be replaced by if(player1), if (player1 == true) { player1 = false; } else { player1 = true; } can be replaced by player1 = !player1;

That's it on top of my head

Answer 2

After re-working my code following Edwin's advice, I would liketo split it in 4 different classes (the entire code above sits withing the Pixeso class): the main class, a JFrame constructors class, a TimerListener event handler class and an ImageButtonListener event handler class

My only doubt is if it is a good idea to decouple the event handlers from the class where the frame is built, since my first attempt to do that was unsuccessful - maybe it's a better idea to leave them within the class with the constructor?