2

In Chapter 15 of Liang's Intro to Java Programming (7th ed.), he introduces a program to make a (2-D) ball on a JPanel and enlarge it upon clicking enlarge/shrink buttons. I've modified the program so that it also 1) enlarges/shrinks the ball if the user clicks/option+clicks, 2) allows you to pick the color of the ball by pressing a button, and 3) allows you to move the circle by dragging it with your mouse.

The last modification is what was giving me trouble for a while, because I wanted to center the ball at the beginning, but then allow the user to move the ball around with the mouse. The solution I came up with was to have the paintComponent method only set the x- and y-coordinates of the ball relative to getWidth() and getHeight() the first time it paints. To do that, I added a paintCount variable to the BallCanvas class and made an if statement so that it would only execute the first time around. When I was trying to figure out how to do this initially, I saw other solutions, like the ones given here: Why can't I access my panel's getWidth() and getHeight() functions? , but I find my solution much simpler.

So the question is: is what I did considered bad coding style? Would a professional programmer scoff at this solution? Or is it OK?

More importantly, is there a better (but also, relatively simple) way to do this that doesn't involve setting up a counter?

Here are the relevant bits of code:

The beginning of BallCanvas:

public static class BallCanvas extends JPanel {

    private int radius = 20;
    private Color color = Color.BLACK;
    private int ballX;
    private int ballY;
    private int paintCount = 0;

    ...

The move method (which responds to a MouseDragged event):

public void move(MouseEvent e){

        ballX = e.getX() - radius;
        ballY = e.getY() - radius;
        repaint();

}

The paintComponent method:

protected void paintComponent(Graphics g){

        super.paintComponent(g);
        g.setColor(color);
        if(paintCount < 1){
            ballX = getWidth()/2 - radius;
            ballY = getHeight()/2 - radius;
        }
        g.fillOval(ballX, ballY, 2*radius, 2*radius);
        paintCount++;

}

Full program:

// Reference: Liang's Intro to Java Programming

import javax.swing.*;
import java.awt.*;
import java.awt.event.*;

public class ControlBall extends JFrame{

    private JButton jbtRed = new JButton("Red");
    private JButton jbtGreen = new JButton("Green");
    private JButton jbtBlue = new JButton("Blue");
    private JButton jbtBlack = new JButton("Black");
    private BallCanvas canvas = new BallCanvas();
    private JMenuBar menuBar = new JMenuBar();
    private JMenu menu = new JMenu("Edit");
    private JMenuItem miEnlarge = new JMenuItem("Enlarge");
    private JMenuItem miShrink = new JMenuItem("Shrink");

    public ControlBall(){

        menuBar.add(menu);
        menu.add(miEnlarge);
        menu.add(miShrink);

        JPanel panel = new JPanel();
        panel.add(jbtRed);
        panel.add(jbtGreen);
        panel.add(jbtBlue);
        panel.add(jbtBlack);

        this.add(canvas, BorderLayout.CENTER);
        this.add(panel, BorderLayout.SOUTH);
        this.add(menuBar, BorderLayout.NORTH);

        jbtRed.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e){
                canvas.setColor(Color.RED);
            }
        });

        jbtGreen.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e){
                canvas.setColor(Color.GREEN);
            }
        });

        jbtBlue.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e){
                canvas.setColor(Color.BLUE);
            }
        });

        jbtBlack.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e){
                canvas.setColor(Color.BLACK);
            }
        });

        miEnlarge.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e){ 
                canvas.enlarge();
            }
        });

        miShrink.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e){ 
                canvas.shrink();
            }
        });

        canvas.addMouseListener(new MouseListener() {
            public void mouseClicked(MouseEvent e){
                canvas.changeSize(e);
            }
            public void mousePressed(MouseEvent e){}
            public void mouseReleased(MouseEvent e){}
            public void mouseEntered(MouseEvent e){}
            public void mouseExited(MouseEvent e){}
        });

        canvas.addMouseMotionListener(new MouseMotionAdapter() {

            public void mouseDragged(MouseEvent e) {

                canvas.move(e);

            }
        });

    }

    public static void main(String[] args){

        JFrame frame = new ControlBall();
        frame.setTitle("ControlBall");
        frame.setLocationRelativeTo(null);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setSize(400, 200);
        frame.setVisible(true);

    }

    public static class BallCanvas extends JPanel {

        private int radius = 20;
        private Color color = Color.BLACK;
        private int ballX;
        private int ballY;
        private int paintCount = 0;

        public BallCanvas(){

            System.out.println(getWidth() + " " + getHeight());

        }

        public BallCanvas(int initialRadius){

            radius = initialRadius;

        }

        public void setColor(Color color){

            this.color = color;
            repaint();

        }

        public void changeSize(MouseEvent e){

            int numClicks = e.getClickCount();

            if(e.isAltDown()){
                if(radius >= 6){
                    this.radius -= 5*numClicks;
                } else{
                    // do nothing
                }
            } else{

                this.radius += 5*numClicks;
            }

            repaint();

        }

        public void enlarge(){

            this.radius += 5;
            repaint();

        }

        public void shrink(){

            if(radius >= 10){
                this.radius -= 5;
            }
            repaint();
        } 

        public void move(MouseEvent e){

            ballX = e.getX() - radius;
            ballY = e.getY() - radius;
            repaint();

        }

        protected void paintComponent(Graphics g){

            super.paintComponent(g);
            g.setColor(color);
            if(paintCount < 1){
                ballX = getWidth()/2 - radius;
                ballY = getHeight()/2 - radius;
            }
            g.fillOval(ballX, ballY, 2*radius, 2*radius);
            paintCount++;

        }

    }

}
Community
  • 1
  • 1
Chris Middleton
  • 5,654
  • 5
  • 31
  • 68
  • 2
    I wouldn't consider it bad programming style. But then again, I'm not a professional. This question would likely elicit opinion-based, instead of fact-based, answers, so I'm voting to close. – Jeffrey Aug 02 '13 at 20:01
  • Perhaps if I change the focus to "is there some better way that you would do it?" Would that be a more appropriate question? – Chris Middleton Aug 02 '13 at 20:03
  • +1 for [sscce](http://sscce.org/) – trashgod Aug 02 '13 at 20:36
  • @ChrisMiddleton Maybe, but then it would probably fall under the category of "too broad": There are either too many possible answers, or good answers would be too long for this format. Please add details to narrow the answer set or to isolate an issue that can be answered in a few paragraphs. – Jeffrey Aug 02 '13 at 20:36
  • @Jeffrey: makes a compelling case; I think you can focus on the preferred size aspect of the problem. – trashgod Aug 02 '13 at 20:50
  • @ChrisMiddleton : Instead of putting an `if` block inside the `paintComponent(...)`, why not you simply take those two statements, i.e. `ballX = ... and ballY = ...` inside the constructor of `BallConvas` class, so that they will be initialized with that value, then latter on as you change them by mouse click, the `paintComponent` will reciprocate with the same effect. Watch this [example](http://stackoverflow.com/a/9852739/1057230), where these initial values are set to `0`, inside the constructor, in your case, you have to change to to the lines already written in `paintComponent()` method. – nIcE cOw Aug 03 '13 at 03:16
  • @nIcEcOw, I tried doing that initially, but the problem is that the frame is not yet visible when the ball is created, so putting them in the constructor will set the ball's initial coordinates both to zero (since the height and width are zero at that point). – Chris Middleton Aug 03 '13 at 23:32
  • @ChrisMiddleton : That means this setting must happen after the call to `pack()` on the parent container :-) – nIcE cOw Aug 04 '13 at 03:09

1 Answers1

4

Several things merit attention:

  • Override getPreferredSize() to establish the panel's initial geometry.

  • Use that geometry to establish the ball's initial position.

  • Invoke pack() and then set the location & visibility.

  • Use Action to encapsulate code shared by menus and controls.

  • Use adapters consistently.

  • Use initial threads correctly.

  • See this Q&A, which examines a related example from several perspectives.

image

import javax.swing.*;
import java.awt.*;
import java.awt.event.*;

public class ControlBall extends JFrame {

    private JButton jbtRed = new JButton("Red");
    private JButton jbtGreen = new JButton("Green");
    private JButton jbtBlue = new JButton("Blue");
    private JButton jbtBlack = new JButton("Black");
    private BallCanvas canvas = new BallCanvas();
    private JMenuBar menuBar = new JMenuBar();
    private JMenu menu = new JMenu("Edit");
    private JMenuItem miEnlarge = new JMenuItem("Enlarge");
    private JMenuItem miShrink = new JMenuItem("Shrink");

    public ControlBall() {

        menuBar.add(menu);
        menu.add(miEnlarge);
        menu.add(miShrink);

        JPanel panel = new JPanel();
        panel.add(jbtRed);
        panel.add(jbtGreen);
        panel.add(jbtBlue);
        panel.add(jbtBlack);

        this.add(canvas, BorderLayout.CENTER);
        this.add(panel, BorderLayout.SOUTH);
        this.add(menuBar, BorderLayout.NORTH);

        jbtRed.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                canvas.setColor(Color.RED);
            }
        });

        jbtGreen.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                canvas.setColor(Color.GREEN);
            }
        });

        jbtBlue.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                canvas.setColor(Color.BLUE);
            }
        });

        jbtBlack.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                canvas.setColor(Color.BLACK);
            }
        });

        miEnlarge.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                canvas.enlarge();
            }
        });

        miShrink.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                canvas.shrink();
            }
        });

        canvas.addMouseListener(new MouseAdapter() {
            @Override
            public void mouseClicked(MouseEvent e) {
                canvas.changeSize(e);
            }

            @Override
            public void mouseDragged(MouseEvent e) {
                canvas.move(e);
            }
        });
    }

    public static void main(String[] args) {

        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {
                JFrame frame = new ControlBall();
                frame.setTitle("ControlBall");
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.pack();
                frame.setLocationRelativeTo(null);
                frame.setVisible(true);
            }
        });

    }

    public static class BallCanvas extends JPanel {

        private static final int SIZE = 400;
        private int radius = 20;
        private Color color = Color.BLACK;
        private int ballX = SIZE / 2 - radius;
        private int ballY = SIZE / 2 - radius;

        public BallCanvas() {
            System.out.println(getWidth() + " " + getHeight());
        }

        public BallCanvas(int initialRadius) {
            radius = initialRadius;
        }

        public void setColor(Color color) {
            this.color = color;
            repaint();
        }

        public void changeSize(MouseEvent e) {

            int numClicks = e.getClickCount();

            if (e.isAltDown()) {
                if (radius >= 6) {
                    this.radius -= 5 * numClicks;
                } else {
                    // do nothing
                }
            } else {

                this.radius += 5 * numClicks;
            }

            repaint();

        }

        public void enlarge() {

            this.radius += 5;
            repaint();

        }

        public void shrink() {

            if (radius >= 10) {
                this.radius -= 5;
            }
            repaint();
        }

        public void move(MouseEvent e) {

            ballX = e.getX() - radius;
            ballY = e.getY() - radius;
            repaint();

        }

        @Override
        protected void paintComponent(Graphics g) {

            super.paintComponent(g);
            g.setColor(color);
            g.fillOval(ballX, ballY, 2 * radius, 2 * radius);

        }

        @Override
        public Dimension getPreferredSize() {
            return new Dimension(SIZE, SIZE);
        }
    }
}
Community
  • 1
  • 1
trashgod
  • 203,806
  • 29
  • 246
  • 1,045
  • 2
    See also this [answer](http://stackoverflow.com/a/5312702/230513) on dragging objects; note how the example's author failed to use adapters consistently. :-) – trashgod Aug 02 '13 at 20:51
  • Wow!! Thank you so much for the very, very detailed answer. It's going to take me a little while to process everything. I might bother you with a question or two if it doesn't click. – Chris Middleton Aug 03 '13 at 23:28