5
\$\begingroup\$

I've created a program where the user is, at first, presented with a window with a yellow circle (random size, random coordinates) and a Start button. Once the user presses Start, the circle, when hovered over, will turn red in color and when it is clicked, another circle (random size, random coordinates) will be generated and this process is repeated (without the Start button).

The circle is generated with a random size of minimum 10 and maximum 100. The circle will be fully visible in the JFrame.

Is there any room for improvement?

import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JButton;
import javax.swing.UIManager;
import javax.swing.UnsupportedLookAndFeelException;

import java.awt.EventQueue;
import java.awt.Graphics;
import java.awt.Dimension;
import java.awt.Color;

import java.awt.geom.Ellipse2D;

import java.awt.event.MouseEvent;
import java.awt.event.MouseAdapter;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;

import java.util.Random;

public class ClickGame {
    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                try {
                    UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
                } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex) {
                    ex.printStackTrace();
                    System.exit(-1);
                }

                JFrame frame = new JFrame("Game - Click the circle!");
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.add(new Panel());
                frame.pack();
                frame.setLocationRelativeTo(null);
                frame.setResizable(false);
                frame.setVisible(true);
            }
        });
    }
}

class Panel extends JPanel {   /* Main panel of Click game */

    Ellipse2D.Double circle;

    boolean hover = false;

    int randomSize = 0; 
    int randomPositionX = 0;
    int randomPositionY = 0;

    public final static int MAX_SIZE = 100;
    public final static int MIN_SIZE = 10;

    public final static int WIDTH    = 500;
    public final static int HEIGHT   = 500;

    private Random rand = new Random();

    public Panel() {
        init();
        generateRandom();
    }

    public void init() {
        final JButton start = new JButton("Start");
        start.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e)
            {
                addMouseMotionListener(new MouseAdapter(){
                    public void mouseMoved(MouseEvent e) {
                        if(circle.contains(e.getPoint()))
                            hover = true;
                        else
                            hover = false;
                        repaint();
                    }
                });

                addMouseListener(new MouseAdapter(){
                    public void mouseClicked(MouseEvent e) {
                        if(circle.contains(e.getPoint()))
                            generateRandom();
                    }
                });
                remove(start);
                repaint();
            }
        });
        add(start);
    }

    public void generateRandom() {
        hover = false;
        randomSize = MIN_SIZE + (rand.nextInt(MAX_SIZE - MIN_SIZE) + 1);

        randomPositionX = rand.nextInt(WIDTH - randomSize);
        randomPositionY = rand.nextInt(HEIGHT - randomSize);
        circle = new Ellipse2D.Double(randomPositionY, randomPositionX, randomSize, randomSize);

        repaint();
    }

    @Override
    public Dimension getPreferredSize() {
        return new Dimension(WIDTH, HEIGHT);
    }

    @Override
    public void paintComponent(Graphics g) {
        super.paintComponent(g);
        if(hover)
            g.setColor(Color.RED);
        else
            g.setColor(Color.YELLOW);
        g.fillOval(randomPositionY, randomPositionX, randomSize, randomSize);
    }

}
\$\endgroup\$
3
\$\begingroup\$

Using an IDE, some changes are suggested automatically. For example: Runnable and ActionListener can be replaced with Lambdas,

        if(circle.contains(e.getPoint()))
            hover = true;
        else
            hover = false;

can be simplified to

hover = circle.contains(e.getPoint());

Also, is it really intended that in Ellipse2D.Double(randomPositionY, randomPositionX, randomSize, randomSize) the first parameter is Y?

Usually if/else statements are used with braces.

In such small classes, the distinction between UI logic and business logic may not seem important, but as programs grow, it also gets more important. The Methods in your MouseListener seem to contain logic which could be extracted to a class or method. Although I am not a Swing expert, IIRC the repaint() method has to be called from the Event-dispatching thread, and could be therefore moved from generateRandom() into the Listener-methods where they are guaranteed to be called from the Event-dispatching thread.

The logic for generating a new circle could be refactored into an own class like this:

class CircleGenerator{
    public final static int MAX_SIZE = 100;
    public final static int MIN_SIZE = 10;
    private Random rand = new Random();

    public Ellipse2D.Double generateRandomCircle() {

        int randomSize = MIN_SIZE + (rand.nextInt(MAX_SIZE - MIN_SIZE) + 1);

        int randomPositionX = rand.nextInt(WIDTH - randomSize);
        int randomPositionY = rand.nextInt(HEIGHT - randomSize);
        return new Ellipse2D.Double(randomPositionX, randomPositionY, randomSize, randomSize);
    }
}
\$\endgroup\$

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.