Switch between drawing a rectangle and by freehand

Question

I have some issues with my paint program in Java.

I have a JComboBox where I can choose to draw either a rectangle or by freehand. The objects are added to an ArrayList. I want to be able to switch between drawing a rectangle and by free hand, and then back to drawing a rectangle, and then by free hand... and so on.

If I do that as the code looks like now, it first draws rectangles fine and then when I switch to free hand it draws lines fine, but then when I switch back to rectangles it still draws lines (or sometimes lines together with weird looking rectangles). The more I switch the weirder it gets.

Can anyone see what is wrong with the code, because I can't?

public abstract class Draw  {
    public int startX, startY, endX, endY, width, height, w, h;
    public String color = "Black";


    public Draw(int startX, int startY, int width, int height) {
        this.startX = startX;
        this.startY = startY;
        this.width = width;
        this.height = height;
    }

    public abstract void draw(Graphics2D g);       

    public int getX() {
        return startX;
    }

    public void setX(int startX) {
        this.startX = startX;
    }

    public int getY() {
        return startY;
    }

    public void setY(int startY) {
        this.startY = startY;
    }

    public int getWidth() {
        return width;
    }

    public void setWidth(int width) {
        this.width = width;
    }

    public int getHeight() {
        return height;
    }

    public void setHeight(int height) {
        this.height = height;
    }

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



public class Rectangle extends Draw {

    public Rectangle(int x, int y, int width, int height) {
        super(x, y, width, height);
    }        

    @Override
    public void draw(Graphics2D g2) {
        g2.drawRect(getX(), getY(), getWidth(), getHeight());
    }  
}

public class FreeHand extends Draw {

     public FreeHand(int x, int y, int width, int height) {
        super(x, y, width, height);
    }

    @Override
    public void draw(Graphics2D g2) {            
        g2.drawLine(getX(), getY(), getWidth(), getHeight());
    }
}

public class PaintProgram extends JFrame implements ActionListener {

    public ArrayList<Draw> shapeList = new ArrayList<>();   
    int startX, startY, endX, endY, w, h;
    private JPanel topPanel;   
    private JPanel bottomPanel;
    private JPanel leftPanel;
    private JPanel rightPanel;
    private JComboBox comboBox;
    private final String[] boxOptions = new String[] {"Rectangle", "Freehand"};   
    Container cp = getContentPane();
    private int count = 0;


    public JavaApplication30(String title) {
        super(title);
        this.setLayout(new BorderLayout());
        this.setDefaultCloseOperation(DO_NOTHING_ON_CLOSE);
        this.setLocationRelativeTo(null);        
        this.setSize(840, 500);     
        this.initComponents();
        this.setVisible(true);            
    }


    private void initComponents() {

        cp.setBackground(Color.WHITE);

        comboBox = new JComboBox(boxOptions);
        topPanel = new JPanel();     
        bottomPanel = new JPanel(new GridLayout(1,2));  
        rightPanel = new JPanel(new FlowLayout(FlowLayout.RIGHT));
        leftPanel = new JPanel(new FlowLayout(FlowLayout.LEFT));

        comboBox.setSelectedIndex(0);          
        comboBox.addActionListener(this); 

        topPanel.setPreferredSize(new Dimension(0,40));        
        bottomPanel.setPreferredSize(new Dimension(0,30));                       
        bottomPanel.setBackground(Color.LIGHT_GRAY);        

        topPanel.add(comboBox);
        bottomPanel.add(leftPanel);        
        bottomPanel.add(rightPanel);

        this.add(topPanel, BorderLayout.PAGE_START);          
        this.add(bottomPanel, BorderLayout.PAGE_END);                  
    }


    @Override
    public void paint(Graphics g) {
        if(count == 0) {
            cp.repaint();
        }
        Graphics2D g2 = (Graphics2D) g;

        for (Draw d : shapeList) {
            d.draw(g2);
        }        

        g2.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON);

        g2.setStroke(new BasicStroke(1));

        if (startX != 0 && startY != 0 && endX != 0 && endY != 0) {

            int width = Math.abs(startX - endX);
            int height = Math.abs(startY - endY);

            int minX = Math.min(startX, endX);
            int minY = Math.min(startY, endY);               

            Rectangle r = new Rectangle(minX, minY, width, height);

            g2.setPaint(Color.WHITE);
            g2.fillRect(r.getX(), r.getY(), r.getWidth(), r.getHeight()); 
            r.setColor(pickedColor);

            r.draw(g2);
        }
    }


    @Override
    public void actionPerformed(ActionEvent e) {

        count++;

        if (e.getSource().equals(comboBox)) {    

            JComboBox cb = (JComboBox)e.getSource();

            if (cb.getSelectedItem().equals("Rectangle")) {

                this.addMouseListener(new MouseAdapter() {    

                    @Override
                    public void mousePressed(MouseEvent e) {                   
                        startX = e.getX();     
                        startY = e.getY(); 

                        endX = startX;  
                        endY = startY;  
                        repaint(); 
                    }

                    @Override
                    public void mouseReleased(MouseEvent e) {
                        endX = e.getX();
                        endY = e.getY();

                        int width = Math.abs(startX - endX);
                        int height = Math.abs(startY - endY);

                        int minX = Math.min(startX, endX);
                        int minY = Math.min(startY, endY);               

                        Rectangle r =  new Rectangle(minX, minY, width, height);
                        shapeList.add(r);
                        r.setColor(pickedColor);

                        startX = 0;
                        startY = 0;
                        endX = 0;
                        endY = 0;
                        repaint();
                    }
                });    

                this.addMouseMotionListener(new MouseMotionAdapter() {
                    @Override
                    public void mouseDragged(MouseEvent e) {
                        endX = e.getX();
                        endY = e.getY();
                        repaint();
                    }
                });
            }            


            else if (cb.getSelectedItem().equals("Freehand")) {

                this.addMouseListener(new MouseAdapter() {                   
                    @Override
                    public void mousePressed(MouseEvent e) {                   
                        startX = e.getX();
                        startY = e.getY();                       
                        addCoordinate(startX, startY);
                    }
                });

                this.addMouseMotionListener(new MouseMotionAdapter() {
                    @Override
                    public void mouseDragged(MouseEvent e) {                
                        Graphics g = getGraphics();    
                        Graphics2D g2 = (Graphics2D) g;         

                        FreeHand fh =  new FreeHand(startX, startY, e.getX(), e.getY());
                        shapeList.add(fh);
                        fh.setColor(pickedColor);
                        fh.draw(g2);                    
                        startX = e.getX();
                        startY = e.getY();                         
                    }
                });    
            }
        }
    }                 

    public static void main(String args[]) {
        new PaintProgram("Paint");
    }
}

Answer 1

You add MouseListeners but you do not remove them. Every time you choose something in the combobox, a new listener is added. So when you draw something every listener is applied and weird stuff will happen.

You should remove the previous MouseListener before adding a new one. You might have to remember it in an instance variable.

Alternatively, you can add all listeners at the start, but check the value of the combobox inside the listener. If the value does not correspond to what the listener is for, it should do nothing.

EDIT: Here is how you can remove all listeners

    for (MouseListener listener : this.getMouseListeners()) {
        this.removeMouseListener(listener);
    }
    for (MouseMotionListener listener : this.getMouseMotionListeners()) {
        this.removeMouseMotionListener(listener);
    }

Put this code in before you add the new listeners in the actionPerformed() method

Answer 2

As was stated here and here previously, do not add MouseListeners within your ActionListener, instead, create a single MosueListener and determine what you want to do based on the currently selected item.

Basically, you keep adding a new MouseListener each time actionPerformed is called...they are accumulating...

A solution would be to use a single MouseListener and a factory of some kind...

Start by defining the factory interface...

public interface DrawFactory {

    public Draw createDrawing(int x, int y, int width, int height, Color color);
    public void addPoint(Draw draw, int x, int y);

}

Create a implementation of the factory for each type of shape you want to draw...

public class RectangleFactory implements DrawFactory {

    @Override
    public Draw createDrawing(int x, int y, int width, int height, Color color) {
        return new Rectangle(x, y, width, height);
    }

    @Override
    public void addPoint(Draw draw, int x, int y) {
        // Does nothing...
    }

    @Override
    public boolean isMutable() {
        return false;
    }

    @Override
    public String getName() {
        return "Rectangle";
    }

    @Override
    public String toString() {
        return getName();
    }

}

public class FreeHandFactory implements DrawFactory {

    @Override
    public Draw createDrawing(int x, int y, int width, int height, Color color) {
        return new FreeHand(x, y, width, height);
    }

    @Override
    public void addPoint(Draw draw, int x, int y) {
        if (draw instanceof FreeHand) {
            FreeHand fh = (FreeHand)draw;
            //fh.addPoint(x, y);
        }
    }

    @Override
    public boolean isMutable() {
        return true;
    }

    @Override
    public String getName() {
        return "Free Hand";
    }

    @Override
    public String toString() {
        return getName();
    }

}

Next, create a custom component that extends from JPanel which will act as the primary drawing surface, this will be repsonsible for monitoring the MouseLstener and painting the Draw instances, as was mentioned here

public class DrawSurface extends JPanel {

    private DrawFactory factory;
    private Draw currentDraw;
    private List<Draw> shapeList = new ArrayList<>();
    private Color drawColor;

    public DrawSurface() {
        shapeList = new ArrayList<>(25);
        MouseAdapter ma = new MouseAdapter() {

            private Point pressPoint;

            @Override
            public void mousePressed(MouseEvent e) {
                pressPoint = e.getPoint();
            }

            @Override
            public void mouseReleased(MouseEvent e) {
                DrawFactory factory = getDrawFactory();
                if (factory != null) {
                    Point p = e.getPoint();
                    if (factory.isMutable() && currentDraw != null) {
                        factory.addPoint(currentDraw, p.x, p.y);
                    } else {
                        int x = Math.min(p.x, pressPoint.x);
                        int y = Math.min(p.y, pressPoint.y);

                        int width = Math.abs(p.x - pressPoint.x);
                        int height = Math.abs(p.y - pressPoint.y);

                        Draw draw = factory.createDrawing(x, y, width, height, getDrawColor());
                        shapeList.add(draw);
                        if (factory.isMutable()) {
                            currentDraw = draw;
                        }
                    }
                }
            }
        };
    }

    public DrawFactory getDrawFactory() {
        return factory;
    }

    public void setDrawFactory(DrawFactory factory) {
        this.factory = factory;
        currentDraw = null;
    }

    public Color getDrawColor() {
        return drawColor;
    }

    public void setDrawColor(Color drawColor) {
        this.drawColor = drawColor;
    }

    @Override
    protected void paintComponent(Graphics g) {
        super.paintComponent(g);
        Graphics2D g2d = (Graphics2D) g.create();
        for (Draw draw : shapeList) {
            draw.draw(g2d);
        }
        g2d.dispose();
    }

    public Dimension getPreferredSize() {
        return new Dimension(200, 200);
    }

}

Next, change your boxOptions from String to DrawFactory, this will make it easier to determine which factory you should use. Don't forget to add a reference to the DrawSurface

private final DrawFactory[] boxOptions = new DrawFactory[]{new RectangleFactory(), new FreeHandFactory()};
private DrawSurface drawSurface;

In your initComponents create a new instance of DrawSurface and add it to your frame...

private void initComponents() {
    //...    
    drawSurface = new DrawSurface();
    this.add(drawSurface);
}

Change your actionPerformed method to look more like...

@Override
public void actionPerformed(ActionEvent e) {
    count++;
    drawSurface.setDrawFactory((DrawFactory)comboBox.getSelectedItem());
}

Not sure how you are determining the current color as you example code is incomplete, but basically, you want to set the drawColor of the DrawSurface similarly.

Get rid of the paint method in the PaintProgram as you shouldn't be overriding the paint method of top level containers, which you've been advised against at least once, if not twice.

The point of all this is simple, when you want to add a new "drawing shape", you create a Draw and DrawFactory for it and add the factory to the combo box ... work done...