2
\$\begingroup\$

My classmates and I were given an assignment to make a 'Paint' application (similar to Microsoft Paint).

There is a color selection JSlider with a connected textbox to each. My code works fine, but it has some bugs in it. Whenever I draw a shape on the panel and change its color, the background of the panel changes to the JSlider panel.

import javax.swing.*;
import javax.swing.event.*;
import java.awt.*;
import java.awt.event.*;
import java.util.*;
public class MyColorChoose extends JFrame implements ChangeListener, KeyListener, ActionListener, MouseListener, MouseMotionListener
{
    JLabel red, green, blue;
    JTextField TRed, TGreen, TBlue;
    JMenu men = new JMenu("File");
    JMenu op = new JMenu("Options");
    JRadioButton square, oval, tri, rect;
    JMenuItem close = new JMenuItem("Exit");
    JMenuBar mBar = new JMenuBar();

    int px, py, rx, ry;

    ColorChooseRectangle sh = new ColorChooseRectangle();

    public int rvalue = 0, gvalue = 0, bvalue = 0;
    public int minR = 0, minG = 0, minB = 0;
    public int setR = 127, setG = 127, setB = 127;
    public int maxR = 255, maxG = 255, maxB = 255;

    Color fill;

    JPanel panel = new JPanel();

    JSlider SRed = new JSlider(JSlider.HORIZONTAL, minR, maxR, setR);
    JSlider SGreen = new JSlider(JSlider.HORIZONTAL, minG, maxG, setG);
    JSlider SBlue = new JSlider(JSlider.HORIZONTAL, minB, maxB, setB);

    public MyColorChoose()
    {
        close.setMnemonic(KeyEvent.VK_X);
        close.addActionListener(this);
        men.add(close);
        mBar.add(men);

        SRed.setPaintTicks(true);
        SRed.setMinorTickSpacing(1);
        SRed.setMajorTickSpacing(50);
        SRed.setPaintLabels(true);

        SGreen.setPaintTicks(true);
        SGreen.setMinorTickSpacing(1);
        SGreen.setMajorTickSpacing(50);
        SGreen.setPaintLabels(true);

        SBlue.setPaintTicks(true);
        SBlue.setMinorTickSpacing(1);
        SBlue.setMajorTickSpacing(50);
        SBlue.setPaintLabels(true);

        red = new JLabel("Red");
        green = new JLabel("Green");
        blue = new JLabel("Blue");
        TRed = new JTextField(10);
        TGreen = new JTextField(10);
        TBlue = new JTextField(10);

        TRed.setText(Integer.toString(SRed.getValue()));
        TRed.addKeyListener(this);
        TBlue.setText(Integer.toString(SBlue.getValue()));
        TBlue.addKeyListener(this);
        TGreen.setText(Integer.toString(SGreen.getValue()));
        TGreen.addKeyListener(this);

        fill = new Color(SRed.getValue(), SGreen.getValue(), SBlue.getValue());
        sh.setChange(fill);

        JPanel PRed = new JPanel(new GridLayout(3,1));
        PRed.add(red);
        PRed.add(TRed);
        PRed.add(SRed);

        JPanel PGr = new JPanel(new GridLayout(3,1));
        PGr.add(green);
        PGr.add(TGreen);
        PGr.add(SGreen);

        JPanel PBlue = new JPanel(new GridLayout(3,1));
        PBlue.add(blue);
        PBlue.add(TBlue);
        PBlue.add(SBlue);

        JPanel left = new JPanel(new GridLayout(3,1));
        left.add(PRed);
        left.add(PGr);
        left.add(PBlue);

        sh.addMouseListener(this);
        sh.addMouseMotionListener(this);

        add(mBar, BorderLayout.NORTH);
        add(left, BorderLayout.WEST);
        add(sh, BorderLayout.CENTER);

        SRed.addChangeListener(new ChangeListener()
        {
            public void stateChanged(ChangeEvent e)
            {
                TRed.setText(String.valueOf(SRed.getValue()));

                gvalue = Integer.parseInt(TGreen.getText());
                bvalue = Integer.parseInt(TBlue.getText());

                fill = new Color(SRed.getValue(), gvalue, bvalue);
                sh.setChange(fill);
            }
        });

        SGreen.addChangeListener(new ChangeListener()
        {
            public void stateChanged(ChangeEvent e)
            {
                TGreen.setText(String.valueOf(SGreen.getValue()));

                rvalue = Integer.parseInt(TRed.getText());
                bvalue = Integer.parseInt(TBlue.getText());

                fill = new Color(rvalue, SGreen.getValue(), bvalue);
                sh.setChange(fill);
            }
        });

        SBlue.addChangeListener(new ChangeListener()
        {
            public void stateChanged(ChangeEvent e)
            {
                TBlue.setText(String.valueOf(SBlue.getValue()));

                rvalue = Integer.parseInt(TRed.getText());
                gvalue = Integer.parseInt(TGreen.getText());

                fill = new Color(rvalue, gvalue, SBlue.getValue());
                sh.setChange(fill);
            }
        });
    }

    public void stateChanged(ChangeEvent e){}

    //KEYBOARD LISTENER
    public void keyTyped(KeyEvent e)
    {
        if(e.getKeyCode() == KeyEvent.VK_ENTER)
        {
            rvalue = Integer.parseInt(TRed.getText());
            gvalue = Integer.parseInt(TGreen.getText());
            bvalue = Integer.parseInt(TBlue.getText());

            SRed.setValue(rvalue);
            SGreen.setValue(gvalue);
            SBlue.setValue(bvalue);

            fill = new Color(rvalue, gvalue, bvalue);

            sh.setChange(fill);
        }            
    }

    public void keyPressed(KeyEvent e)
    {
        if(e.getKeyCode() == KeyEvent.VK_ENTER)
        {
            rvalue = Integer.parseInt(TRed.getText());
            gvalue = Integer.parseInt(TGreen.getText());
            bvalue = Integer.parseInt(TBlue.getText());

            SRed.setValue(rvalue);
            SGreen.setValue(gvalue);
            SBlue.setValue(bvalue);

            fill = new Color(rvalue, gvalue, bvalue);

            sh.setChange(fill);
        }            
    }

    public void keyReleased(KeyEvent e){}

    //ACTION LISTENER
    public void actionPerformed(ActionEvent e)
    {
        Object sel = e.getSource();
        if(sel == close)
            System.exit(0);
    }

    //MOUSE LISTENER

    public void mouseClicked(MouseEvent e){}
    public void mouseEntered(MouseEvent e){}
    public void mouseExited(MouseEvent e){}
    public void mouseMoved(MouseEvent e){}
    public void mousePressed(MouseEvent e)
    {
        px = e.getX();
        py = e.getY();
    }
    public void mouseDragged(MouseEvent e)
    {
        rx = e.getX();
        ry = e.getY();

            int w = rx - px;
            int h = ry - py;

            sh.setSize(px, py, w, h);
    }
    public void mouseReleased(MouseEvent e)
    {
        rx = e.getX();
        ry = e.getY();

            int w = rx - px;
            int h = ry - py;

            sh.setSize(px, py, w, h);
    }

    public static void main(String[] args) 
    {
        MyColorChoose m = new MyColorChoose();
        m.setSize(600,440);
        m.setVisible(true);
        m.setDefaultCloseOperation(m.EXIT_ON_CLOSE);
        m.setLocationRelativeTo(null);
    }


}

Drawing class:

import java.awt.*;
import javax.swing.*;
public class ColorChooseRectangle extends JPanel 
{
    public Color change;
    private int X, Y, W, H;
    public void paint(Graphics g) 
    {
        g.setColor(change);
        g.drawRect(X,Y,W,H);
        g.fillRect(X,Y,W,H);
    }

       public void setChange(Color c)
       {
           change = c;
           repaint();
       }

       public void setSize(int x, int y, int w, int h)
       {
           this.X = x;
           this.Y = y;
           this.W = w;
           this.H = h;
           repaint();
       }
}

Some initialized variables are not yet used, but I will use them.

\$\endgroup\$
0
1
\$\begingroup\$

I ran your exact code on my computer, I'm a little confused on what your actual problem is, but if you're trying to "clear" the panel every time you draw a new shape the code for that would be:

public void paint(Graphics g) 
{
    g.clearRect(0,0,getWidth(),getHeight());
    g.setColor(change);
    g.drawRect(X,Y,W,H);
    g.fillRect(X,Y,W,H);
}

If this isn't what you are looking for, please explain your problem in more detail.

\$\endgroup\$
7
  • \$\begingroup\$ No, actually, i want the frame to retain the shapes I Draw.. anyway, here's the full detail of my problem, I Have drawn a shape on my drawing panel, now, whenever I change the color on the jsliders, the JSlider and its textbox partner appear on the upper left side of the drawing panel,.. Even funny was, whenever I adjust the jslider, the value on the textbox which is adjusting, is also adjusting on the drawing panel! I don't know what is the problem of my code, please help me sir.. \$\endgroup\$
    – Eduard
    Mar 11 '12 at 11:40
  • 1
    \$\begingroup\$ Without very good reasons you shouldn't overwrite paint in Swing, but overwrite paintComponent instead. \$\endgroup\$
    – Landei
    Mar 11 '12 at 17:39
  • \$\begingroup\$ ohhh so you dont want the color on the JPanel to change when you move the slider??? you only want the slider to effect shapes that are about to be drawn? \$\endgroup\$
    – John
    Mar 12 '12 at 4:02
  • \$\begingroup\$ @Lanei you are correct, some people override paint() but it is better to override paintComponent() i agree with you and i know that is correct \$\endgroup\$
    – John
    Mar 12 '12 at 4:03
  • \$\begingroup\$ NO, what I meant is my drawing panel gets messed up by some parts of my frame, (example, I Have drawn a shape, now, whenever i slide the jslider, the jslider image(the slider itself) gets on the drawing panel. how do i fix that? \$\endgroup\$
    – Eduard
    Mar 12 '12 at 14:47
1
\$\begingroup\$

Some notes:

  • Access static fields and methods statically. That means instead of using m.EXIT_ON_CLOSE use JFrame.EXIT_ON_CLOSE
  • Separate the event handler from the gui. JFrame and JPanel subclasses should not implement listeners.
  • Try to keep the scope of your variables as small as possible. If you can make them local variables, do so.
  • Try to keep the access modifiers of your variables as limited as possible. If you can make them private, do so. Getters and setters maintain encapsulation.
  • JFrame has a setJMenuBar(JMenuBar) function that you should use for your menu bar.
  • For extendability, subclass the content pane rather than the top level container like this:

    public class MyColorChoose extends JPanel {
    
         // all the stuff from MyColorChoose
    
         public static void main(String[] args) {
             JFrame frame = new JFrame();
             frame.setJMenuBar(initMenuBar());
             frame.setContentPane(new MyColorChoose());
             frame.pack();
             frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
             frame.setLocationRelativeTo(null);
             frame.setVisible(true);
         }
    
     }
    

    That way if you suddenly decide to make it an applet, you don't have to duplicate much code. You can also separate out the main function into another class. I'd rather not create the menu bar in the content pane class.

  • NO magic numbers EVER! That means new JPanel(new GridLayout(3, 1)) is unacceptable. Constants are your friends.
  • Use the @Override annotation when overriding methods.
  • Favor key bindings over key listeners. Key listeners are low level,while key bindings are high level. Key bindings look like this:

    component.getInputMap().put(Keystroke.getKeystroke("P"), "pause");
    // if an Action pauseAction has been defined, we can do this:
    component.getActionMap().put("pause", pauseAction);
    
  • JTextFields fire action events when the enter key is pressed. Register an action listener with each of your fields instead of using key listener on them.

  • You panels containing sliders have duplicate code. Refactor them into methods.
\$\endgroup\$

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.