patternjavaMinor
Paint rectangles to canvas using mouse
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() {
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.
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.
should be something like this:
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.