HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Paint rectangles to canvas using mouse

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rectanglesusingpaintcanvasmouse

Problem

Just wanted to post this here to see if anyone has any critiques of my code, if I'm drawing the background image most efficiently, or if I'm doing anything which could benefit from obvious improvements.

RectangleMover

```
import java.awt.BorderLayout;
import java.awt.EventQueue;
import java.awt.Graphics;
import java.awt.Image;
import java.awt.event.MouseEvent;
import java.awt.event.MouseMotionListener;
import java.io.File;
import java.io.IOException;

import javax.swing.JFrame;
import javax.swing.JPanel;

public class RectangleMover {
public static void main(String[] args) {
EventQueue.invokeLater(new Runnable() {
public void run() {
final JFrame frame = new JFrame();

frame.setSize(FRAME_WIDTH, FRAME_HEIGHT);
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.setContentPane(new JPanel(new BorderLayout()) {
/**
*
*/
private static final long serialVersionUID = 1L;

public void paintComponent(Graphics g) {
try {
super.paintComponent(g);
final Image backgroundImage = javax.imageio.ImageIO
.read(new File(
"/Users/langer/Desktop/7923.jpg"));
g.drawImage(backgroundImage, 0, 0, FRAME_WIDTH,
FRAME_HEIGHT, null);
} catch (IOException e) {
e.printStackTrace();
}
}
});
frame.setVisible(true);

frame.setLocationRelativeTo(null);

final RectangleComponent b = new RectangleComponent();
frame.add(b);
frame.addMouseMotionListener(new MouseMotionListener() {

Solution

Don't load the same resource repeatedly

I don't have much time to give a full review but I can point out one big thing that I saw while looking through the code.

public void paintComponent(Graphics g) {
                    try {
                        super.paintComponent(g);
                        final Image backgroundImage = javax.imageio.ImageIO
                                .read(new File(
                                        "/Users/langer/Desktop/7923.jpg"));
                        g.drawImage(backgroundImage, 0, 0, FRAME_WIDTH,
                                FRAME_HEIGHT, null);
                    } catch (IOException e) {
                        e.printStackTrace();
                    }
                }


You shouldn't load and parse the background image in the paint method. This method can be called frequently even though your background image doesn't change. If you try to resize your window you'll see how you reload the background several times per second slowing down your application.

Prefer to create a program instance

Doing so allows you to encapsulate variables to the class instead of keeping them in main or as inline. Also makes your code more readable.

public class RectangleMover {
    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                final JFrame frame = new JFrame();

                frame.setSize(FRAME_WIDTH, FRAME_HEIGHT);
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.setContentPane(new JPanel(new BorderLayout()) {
                    /**
                     * 
                     */
                    private static final long serialVersionUID = 1L;


should be something like this:

public class RectangleMover {

    private final Image background;

    public RectangleMover(){
        background = loadimage();
        final JFrame frame = new JFrame();
        ...
    }

    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                new RectangleMover();
            }});
    }

Code Snippets

public void paintComponent(Graphics g) {
                    try {
                        super.paintComponent(g);
                        final Image backgroundImage = javax.imageio.ImageIO
                                .read(new File(
                                        "/Users/langer/Desktop/7923.jpg"));
                        g.drawImage(backgroundImage, 0, 0, FRAME_WIDTH,
                                FRAME_HEIGHT, null);
                    } catch (IOException e) {
                        e.printStackTrace();
                    }
                }
public class RectangleMover {
    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                final JFrame frame = new JFrame();

                frame.setSize(FRAME_WIDTH, FRAME_HEIGHT);
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.setContentPane(new JPanel(new BorderLayout()) {
                    /**
                     * 
                     */
                    private static final long serialVersionUID = 1L;
public class RectangleMover {

    private final Image background;

    public RectangleMover(){
        background = loadimage();
        final JFrame frame = new JFrame();
        ...
    }


    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                new RectangleMover();
            }});
    }

Context

StackExchange Code Review Q#88127, answer score: 7

Revisions (0)

No revisions yet.