2
\$\begingroup\$

basically I'm learning Java Swing by myself and I'm wondering what is the correct way to wrote code for it? I don't want to have a big class with everything in it and I want to do in the most "correct" way. For example, I have this program right there that it is supposed to paint N rectangles, where N is given in input by the user using a JTextField. Basically you enter the number of rectangles that you want, you click "Ok" and it is supposed to draw that amount of rectangles in a random place into another JPanel that is under the input area by repainting that area. It doesn't really work as intended because the rectangles aren't the same size (as I wanted to) and the repaint method only works when I resize the window.

MainFrame.java

//This is just a class that I use to create the JFrame in the main class, so I can work directly on the JFrame just here
public class MainFrame extends JFrame
{
    public MainFrame(String title, int width, int height)
    {
        super(title);
        this.setSize(width, height);
        this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE); 
    }
}

MainWindow.java

//This is the main class where I'm trying to have as little code as possible 
public class MainWindow
{
   public static void main(String[] args)
   {
       MainFrame frame = new MainFrame("RandomRectangles", 500, 500);

       RectanglesPanel rectangles = new RectanglesPanel(frame);
       JPanel mainPanel = new JPanel(new BorderLayout());
       InputPanel inputPanel = new InputPanel();

           //Is adding the listener here a "good practice" or not? 

       inputPanel.getButton().addActionListener(e ->
       {
                       //adds all the rectangles in the JPanel
           for(int c = 0; c < Integer.parseInt(inputPanel.getTextField().getText()); c++)
           {
               RectangleComponent box = new RectangleComponent();
               rectangles.add(box);
           }
           mainPanel.repaint();    
       });


       mainPanel.add(inputPanel, BorderLayout.NORTH);
       mainPanel.add(rectangles, BorderLayout.CENTER);

       frame.add(mainPanel);
       frame.setVisible(true);
   }
}

RectanglesPanel.java

//This is the panel that I use as a "canvas" to paint the rectangles 
public class RectanglesPanel extends JPanel
{
   public RectanglesPanel(MainFrame frame)
   {
       super(new GridLayout());

       this.setPreferredSize(new Dimension(frame.getWidth() - 100, frame.getHeight() - 100));
       this.setBackground(Color.BLACK);
   }
}

InputPanel.java

//The panel where the input stuff happens, I added those methods because I needed them where I added the listener to the button, but again I don't know if this is a "good practice" or not
public class InputPanel extends JPanel
{
   private JButton button;
   private JLabel label;
   private JTextField text;

   public InputPanel()
   {
       super(new FlowLayout());

       button = new JButton("Ok");
       label = new JLabel("How many rectangles?");
       text = new JTextField(10);

       this.add(label);
       this.add(text);
       this.add(button);
   }

   public JButton getButton()
   {
       return this.button;
   }

   public JTextField getTextField()
   {
       return this.text;
   }

}

RectangleComponent.java

//Here's the rectangles that are a JComponent
public class RectangleComponent extends JComponent
{   
   @Override
   protected void paintComponent(Graphics g)
   {
       super.paintComponent(g);
       Graphics2D g2 = (Graphics2D)g;
       Random random = new Random();
       //I draw them in a random location every time, but supposedly with the same dimensions
       Rectangle box = new Rectangle(random.nextInt(getWidth()), random.nextInt(getHeight()), 30, 20);
       g2.setColor(Color.RED);
       g2.fill(box);
       g2.draw(box);
   }

   @Override
   public Dimension getPreferredSize()
   {
       return new Dimension(30, 20);
   }

   @Override
   public Dimension getMaximumSize()
   {
       return new Dimension(30, 20);
   }

   @Override
   public Dimension getMinimumSize()
   {
       return new Dimension(30, 20);
   }
}

Is there something I can improve in the way I'm writing this code? Also, this is my first question here so let me know if I've done something wrong with the post, I'll edit it.

\$\endgroup\$
  • 1
    \$\begingroup\$ It's because I first posted this question on stackoverflow, but they told me to ask here and I just copy and pasted what I wrote there, my bad. Anyway you're right, I'll edit the post trying to explain myself better \$\endgroup\$ – TheBleccbird Nov 19 '19 at 18:18
  • \$\begingroup\$ Haven't read all your code yet but be aware there's no one single "right way" to write code, including Swing. Often it depends (heavily) on the problem. Just wanted to address that one point. \$\endgroup\$ – markspace Nov 19 '19 at 20:17
3
\$\begingroup\$

Swing code is generally appalling, and that includes the tutorials.

As always in Java, don't go around subclassing classes you don't need to. You wouldn't dare subclass Thread (any more), so don't subclass JFrame or JPanel. However, for JComponent.paintComponent methods is being overridden sensibly, the get method should be replaced by calling the appropriate sets.

What appear to be "objects" in naïve object orientation, should really be just plain functions. However, often processes should be object - think of them of processors.

Classes that are just a collection of fields with get (and possibly set) method, are probably a bad idea. You'll also see get followed only by an action on the gotten value.

Swing (and in practice AWT) should only be used from the AWT Event Dispatch Thread (EDT). main executes in a different thread. So you need to switch threads.

public static void main(String[] args) {
    java.awt.EventQueue.invokeLater(MyApp::swingGo);
}
private static void swingGo() {

Your RectanglePanel attempts to derive its preferred size from the size of a JFrame. That wont update when the window is resized, and looks to be attempting to do something a LayoutManager should be doing.

That repaint should be a revalidate as you are adding components.

I don't think you don't need to both fill and draw the rectangle in the same colour.

Integer.parseInt may throw an exception, so that should be caught rather than thrown (no need to dump the stack tract).

In terms of style: In Java open braces are usually on the same line as the statement they belong to. parseInt could be stored in a local variable, rather than living within the for statement.

| improve this answer | |
\$\endgroup\$
  • 2
    \$\begingroup\$ I have a gut feeling that Swing code is apalling because the tutorials try to cram everything needed in the case into a single class and programmers just follow the instructions. Doing a proper MVC would take too much "blog space". \$\endgroup\$ – TorbenPutkonen Nov 22 '19 at 10:44
  • 1
    \$\begingroup\$ @TorbenPutkonen Proper MVC? Have you seen the arrows if you image google MVC? google.co.uk/search?tbm=isch&q=mvc I'm happy with the new development with some diagrams having four boxes. \$\endgroup\$ – Tom Hawtin - tackline Nov 22 '19 at 13:17

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.