Drawing simple shapes by mouse dragging

Question

I want to make figures of a rectangle and a circle. How can I make the code more elegant?

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

class r extends JPanel
{
      public int x1,x2,y1,y2;
      public static double SWITCH;
      public r()
      {
          setBackground(Color.WHITE);
          addMouseListener(new MouseAdapter()
          {
               public void mousePressed(MouseEvent m)
               {
               x1=m.getX();
               y1=m.getY();
               repaint();
               }
               public void mouseReleased(MouseEvent m)
               {
               x2=m.getX();
               y2=m.getY();
               repaint();
               }
          });
          addMouseMotionListener(new MouseMotionAdapter()
          {
              public void mouseDragged(MouseEvent m)
              {
                 x2=m.getX();
                 y2=m.getY();
                 repaint();
              }
          });
      }
      public void paintComponent(Graphics g)
      { 
          super.paintComponent(g);
          if(SWITCH == 2)
          {
              g.drawRect(x1, y1, x2, y2);
          }
          else if (SWITCH == 3)
          {
              g.drawOval(x1,y1,x2,y2);
          }
          else
          {
              g.drawString("qwe", x1, y1);
          }
      }

  }
  public class q extends JFrame
  {
      public static void main(String[] args)
      {
          q window = new q();
          window.setVisible(true);
          window.setSize(1024, 800);
          window.setDefaultCloseOperation(EXIT_ON_CLOSE);
          Container cont = window.getContentPane();
          cont.setLayout(new GridLayout(2,2));
          r panel = new r();
          JPanel BPanel = new JPanel();
          cont.add(panel);
          cont.add(BPanel);
          BPanel.setBackground(Color.blue);
          JButton button1,button2;
          button1 = new JButton("Rect");
          button2 = new JButton("Oval");
          BPanel.add(button1);
          BPanel.add(button2);
          button1.addActionListener(new ActionListener() {

              public void actionPerformed(ActionEvent e)
              {
                  r.SWITCH = 2;
              }
          });
          button2.addActionListener(new ActionListener() {

              public void actionPerformed(ActionEvent e)
              {
                  r.SWITCH = 3;
              }
          });
      }
  }

Answer 1

Just a few thoughts...

You are using x and y coordinates. Why not take advantage of the Point class? The MouseEvent class has methods that return Point objects, and it would probably make your code more readable. x1 and y1 don't mean anything to me, but you could probably give meaningful names to the points represented by (x1, y1) and (x2, y2).

The use of magic numbers (you storing 2 and 3 into SWITCH, for example) is a poor practice. Use constant ints or (as Landei suggested in the comments) an enum. It will allow you to improve the readable and understandability of the code - at a first read-through, I didn't get what you were doing.

As per Java standards, a class name should begin with a capital letter. Your class is currently called r. You should give it a meaningful name. You might also want to look into other Java standards for code formatting in terms of brace placement, indentation, and so forth. To me, your code is just plain hard to read because of formatting.

Rather than if and switch/case statements, consider leveraging polymorphism in order to draw different shapes. Move the logic to draw different shapes into concrete subclasses, while perhaps maintaining common logic in a superclass.

Answer 2

A couple more thoughts.

Use uppercase when naming a class.

Separate your example into two classes, say DrawPanel for your class that creates the drawing panel and DrawObjects for the class that is creating the drawing objects.

Add a few comments in your code explaining what you are trying to do and why. Use constructors to clean up your code and to make it more readable.

I made a few improvements to the code and put in two classes below, take a look and ask questions.

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

public class DrawPanel extends JFrame
{
 private DrawObjects panel = new DrawObjects();
 private JPanel BPanel = new JPanel();
 private JFrame window = new JFrame();

 //constructor
 DrawPanel(){
    buildGUI(); 
 }

 void buildGUI(){
     window.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
     window.setLayout(new GridLayout(2,2));
     window.add(panel);
     window.add(BPanel);
     BPanel.setBackground(Color.blue);

      //define buttons and add to panel
      JButton rect = new JButton("Rect");
      JButton oval = new JButton("Oval");
      BPanel.add(rect);
      BPanel.add(oval);

      rect.addActionListener(new ActionListener() {
          public void actionPerformed(ActionEvent e)
              {
                  panel.setType(1);
              }
      });

      oval.addActionListener(new ActionListener() {
          public void actionPerformed(ActionEvent e)
              {
              panel.setType(2);
              }
          }); 

      window.setVisible(true);
      window.setSize(1024, 800);     
}

 public static void main(String[] args)
  {
     //create this object
     new DrawPanel();
  }

  }//end class

class two

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

class DrawObjects extends JPanel
{
public int x1,x2,y1,y2;
public int type = 1;//default draw type

public DrawObjects()
{
    init();
}

public void init(){
      setBackground(Color.WHITE);
      addMouseListener(new MouseAdapter()
       {
          public void mousePressed(MouseEvent m)
          {
               x1 = m.getX();
               y1 = m.getY();
               repaint();
          }
          public void mouseReleased(MouseEvent m)
          {
               x2 = m.getX();
               y2 = m.getY();
               repaint();
          }
        });

      addMouseMotionListener(new MouseMotionAdapter()
        {
           public void mouseDragged(MouseEvent m)
              {
                 x2 = m.getX();
                 y2 = m.getY();
                 repaint();
              }
          });
}

public void setType(int arg){
    if(arg == 1){
        type = 1;
    }else if(arg == 2){
        type = 2;
    }
}

  public void paintComponent(Graphics g)
  { 
      super.paintComponent(g);
      if(type == 1)
      {
          g.drawRect(x1,y1,x2,y2);
      }
      else if (type == 2)
      {
          g.drawOval(x1,y1,x2,y2);
      }
  }
    }