Drawing multiple shapes depending on user Java Swing

Question

I am trying to implement a menu based graph - wherein i will represent a vertex by a circle and each circle will have an index provided by the user.The user has to go to File menu and click on Addvertex to create a new node with an index.The Problem though is - The circle is drawn only once - any subsequent clicks to addVertex does not result in drawing of a circle -Can't understand Why...

Here is my code:

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;
import java.io.*;
import java.util.*;

public class FileChoose extends JFrame {

    public FileChoose() {
        JMenuBar l=new JMenuBar();
        JMenu file=new JMenu("File");
        JMenuItem open=new JMenuItem("Addvertex");
        open.addActionListener(new Drawer());
        JMenuItem close=new JMenuItem("Exit");
        close.addActionListener(new ActionListener() {

            public void actionPerformed(ActionEvent e) {
                System.exit(0);
            }
        });
        JMenu tools=new JMenu("Tools");
        file.add(open);
        file.add(close);

        this.setJMenuBar(l);
        l.add(tools);
        l.add(file);

        this.setSize(new Dimension(200, 200));
        this.setLocationRelativeTo(null);
        this.setDefaultCloseOperation(this.EXIT_ON_CLOSE);
    }

    void HelloHere(String p) {
        Draw d = new Draw(p);
        this.add(d);
        setExtendedState(MAXIMIZED_BOTH);
    }

    class Drawer extends JFrame implements ActionListener {

        Integer index;

        public void actionPerformed(ActionEvent e) {
            JPanel pn = new JPanel();
            JTextField jt = new JTextField(5);
            JTextField jt1 = new JTextField(5);
            pn.add(jt);
            pn.add(jt1);
            int result=JOptionPane.showConfirmDialog(null, pn, "Enter the values", JOptionPane.OK_CANCEL_OPTION);
            if (result == JOptionPane.OK_OPTION) {
                index = Integer.parseInt(jt.getText());
                System.out.println(jt1.getText());
            }

            this.setVisible(true);
            this.setSize(500, 500);
            this.setLocationRelativeTo(null);

            HelloHere(index.toString());
        }
    }

    public static void main(String [] args) {
        FileChoose f = new FileChoose();
        f.setVisible(true);
    }
}

class Draw extends JPanel {

    String inp;

    public Draw(String gn) {
        inp = gn;
    }

    @Override
    public void paintComponent(Graphics g) {
        super.paintComponent(g);

        Graphics2D g2 = (Graphics2D) g;
        Random r = new Random();
        int x = r.nextInt(100);

        g2.drawOval(x, x * 2, 100, 100);
        g2.drawString(inp, x + (x / 2), x + (x / 2));
    }
}

please insert your code in the post. – vainolo Nov 12 '12 at 15:37 — vainolo, Nov 12 '12 at 15:37

score 4 · Accepted Answer · edited May 23 '17 at 10:24

Some pointers:

Use EDT for creation and manipulation of Swing Components.
Dont extend JFrame class unnecessarily.
Use anonymous Listeners where possible/allowed

Make sure JFrame#setVisible(..) is the last call on JFrame instance specifically pointing here:

    this.setVisible(true);
    this.setSize(500, 500);
    this.setLocationRelativeTo(null);
    HelloHere(index.toString());

Call pack() on JFrame instance rather than setSize(..)
Do not use multiple JFrames see: The Use of Multiple JFrames: Good or Bad Practice?
The problem you are having is here:
```
Draw d = new Draw(p);
this.add(d);
```
You are creating a new instance of Draw JPanel each time and then overwriting the last JPanel added with the new one (this is default the behavior of BorderLayout). To solve this read below 2 points:
Call revalidate() and repaint() on instance after adding components to show newly added components.
Use an appropriate LayoutManager

As I am not sure of you expected results I have done as much fixing of the code as I could hope it helps:

import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.*;
import javax.swing.*;

public class FileChoose {

    JFrame frame;

    public FileChoose() {
        frame = new JFrame();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        JMenuBar l = new JMenuBar();
        JMenu file = new JMenu("File");
        JMenuItem open = new JMenuItem("Addvertex");

        open.addActionListener(new ActionListener() {
            Integer index;

            @Override
            public void actionPerformed(ActionEvent e) {
                JPanel pn = new JPanel();
                JTextField jt = new JTextField(5);
                JTextField jt1 = new JTextField(5);
                pn.add(jt);
                pn.add(jt1);
                int result = JOptionPane.showConfirmDialog(null, pn, "Enter the values", JOptionPane.OK_CANCEL_OPTION);
                if (result == JOptionPane.OK_OPTION) {
                    index = Integer.parseInt(jt.getText());
                    System.out.println(jt1.getText());
                }
                HelloHere(index.toString());
            }
        });

        JMenuItem close = new JMenuItem("Exit");
        close.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                System.exit(0);
            }
        });

        JMenu tools = new JMenu("Tools");
        file.add(open);
        file.add(close);

        frame.setJMenuBar(l);
        l.add(tools);
        l.add(file);
        frame.pack();

        frame.setLocationRelativeTo(null);
        frame.setVisible(true);
    }

    void HelloHere(String p) {
        Draw d = new Draw(p);
        frame.add(d);
        frame.setExtendedState(JFrame.MAXIMIZED_BOTH);
        frame.revalidate();
        frame.repaint();
    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                FileChoose f = new FileChoose();
            }
        });
    }
}

class Draw extends JPanel {

    String inp;

    public Draw(String gn) {
        inp = gn;
    }

    @Override
    public void paintComponent(Graphics g) {
        super.paintComponent(g);

        Graphics2D g2 = (Graphics2D) g;
        Random r = new Random();
        int x = r.nextInt(100);

        g2.drawOval(x, x * 2, 100, 100);
        g2.drawString(inp, x + (x / 2), x + (x / 2));
    }
}

UPDATE:

Declare this globally in your Draw class: Random r=new Random(); as if you iniate a new Random instance each time you call paintComponent() the distributions will not be significant/random enough

Thanks for all the points you gave me...But,what repaint will do is - remove the existing circle drawn and then draw a new circle as specified by the user - what i want is - the already existing figure should continue to appear and the new circle appears somewhere else in a random location.(Hence the random value i used in my code)..Thanks — 404, Nov 13 '12 at 06:52
@404 You should than create an array of Ovals in your `Draw` class, and each tome a user adds a vertex an oval is added to the array, thus calling `repaint()` will cause all the ovals in the array to be drawn in `paintComponent()` — David Kroukamp, Nov 13 '12 at 07:22
Each time i instantiate a new draw object -the array will be new- with no oval corresponding to it... — 404, Nov 13 '12 at 07:32
@404 well obviously than you wont initiate an new object for it each time, declare and create/initiate the Draw object globally and simply call you add oval method like so: `draw.addOval(int x,int y,int width, int height);` — David Kroukamp, Nov 13 '12 at 07:33
Finally was able to do it - Nothing much - just decalred Draw object globally and called draw.paintcomponent() repeatedly - no need for an array - and no repaints too....Thanks a lot David..i appreciate your help throughout the course of the day..thanks — 404, Nov 13 '12 at 07:56
@404 its a pleasure. As for your comment *and called `draw.paintcomponent()` repeatedly ..* this is not the correct solution. You should call `repaint()` to honor *the `paint()` chain* hence the necessary array of objects to draw. What happens when the `JPanel` decides it needs to `repaint()` or is invoked to repaint through another top level container? all your shapes will be lost — David Kroukamp, Nov 13 '12 at 08:32
@404 please see my updated post and this similar question/answer: http://stackoverflow.com/questions/13358658/paintcomponent-draws-other-components-on-top-of-my-drawing/13359279#13359279 — David Kroukamp, Nov 13 '12 at 10:56

score 2 · Answer 2 · answered Nov 12 '12 at 15:54

Every time the user creates a new node you create a new JPanel that is added to the JFrame of your application. In the absence of a specific layout it uses a BorderLayout. BorderLayout does not allow for more than one component to be added at each location. Your other calls to add are probably ignored.

Drawing multiple shapes depending on user Java Swing

2 Answers2