See the previous iteration: Bare bones painter app in Java.
I have improved my program thanks to all the answers in the initial post. Now I have this:
App.java:
package net.coderodde.javapaint;
import javax.swing.JFrame;
/**
* This class implements an application for simple drawing.
*
* @author Rodion "rodde" Efremov
* @version 1.6 (Jun 8, 2016)
*/
public class App {
private static final String APP_TITLE = "JavaPaint";
private static final int APP_WINDOW_WIDTH = 640;
private static final int APP_WINDOW_HEIGHT = 480;
private final JFrame frame = new JFrame(APP_TITLE);
public App() {
frame.getContentPane()
.add(new PaintCanvas(APP_WINDOW_WIDTH, APP_WINDOW_HEIGHT));
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.pack();
frame.setResizable(false);
frame.setVisible(true);
}
public static void main(final String[] args) {
final App app = new App();
}
}
PaintCanvas.java:
package net.coderodde.javapaint;
import java.awt.BasicStroke;
import java.awt.Canvas;
import java.awt.Color;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import java.awt.event.MouseMotionListener;
import java.awt.image.BufferedImage;
/**
* This class implements a GUI component for drawing.
*
* @author Rodion "rodde" Efremov
* @version 1.6 (Jun 8, 2016)
*/
public class PaintCanvas extends Canvas {
/**
* The radius of the brush in pixels.
*/
private static final int BRUSH_RADIUS = 5;
/**
* The actual image being drawn to.
*/
private final BufferedImage image;
/**
* The graphics context of the above.
*/
private final Graphics2D imageGraphics;
public PaintCanvas(final int width, final int height) {
super();
super.setBounds(0, 0, width, height);
this.image = new BufferedImage(width,
height,
BufferedImage.TYPE_INT_RGB);
this.imageGraphics = this.image.createGraphics();
prepareScreen(width, height);
final PaintCanvasMouseListener listener =
new PaintCanvasMouseListener();
this.addMouseListener(listener);
this.addMouseMotionListener(listener);
}
@Override
public void paint(final Graphics g) {
update(g);
}
@Override
public void update(final Graphics g) {
g.drawImage(this.image, 0, 0, null);
}
private void prepareScreen(final int width, final int height) {
this.imageGraphics.setColor(Color.WHITE);
this.imageGraphics.fillRect(0, 0, width, height);
this.imageGraphics.setColor(Color.BLACK);
this.imageGraphics.setStroke(new BasicStroke(BRUSH_RADIUS));
}
private class PaintCanvasMouseListener
extends MouseAdapter implements MouseMotionListener {
private int previousX;
private int previousY;
@Override
public void mousePressed(final MouseEvent event) {
this.previousX = event.getX();
this.previousY = event.getY();
}
@Override
public void mouseClicked(final MouseEvent event) {
final int x = event.getX();
final int y = event.getY();
PaintCanvas.this.imageGraphics.drawLine(x, y, x, y);
}
@Override
public void mouseDragged(final MouseEvent event) {
final int currentX = event.getX();
final int currentY = event.getY();
PaintCanvas.this.imageGraphics.drawLine(previousX,
previousY,
currentX,
currentY);
PaintCanvas.this.repaint();
this.previousX = currentX;
this.previousY = currentY;
}
}
}
The new version allowed me to paint the following masterpiece:
Please, tell me anything that comes to mind.
1 Answer 1
Constructor Responsibilities
Your App-constructor does two things. It sets up the JFrame's layout and makes it visible. When I want to reuse your app I may want to control when it shows up myself.
No Program should show itself on the screen unsolicitedly. Make the App show only when calling a show()
method and not when it's constructed.
You also don't have to explicitly call super()
. That's done anyways
If this constructor is for a class other than Object, then this constructor will begin with an explicit or implicit invocation of a superclass constructor (using super).
More little things:
Your main-method could be simplified a bit:
public static void main (String[] args) { new App(); }
is sufficient with your current code. The instance created is never used anywhere else.
You're splitting lines at arguments. I personally find that hard to read and would only split if the lines get too long.
You're overqualifying. The mouselistener code doesn't need
PaintCanvas.this.
anywhere. Since the listener is declared in a non-static inner class it automatically has access to the enclosing class fields. (same applies forprepareScreen
)
Your code doesn't have much more to say about it, simply because it's so short. The only thing that I'm wondering is...
Why Build this in Swing?
I don't understand why you'd want to learn swing in this day and age. It's outdated, if not a dinosaur. It's officially deprecated and people starting new Apps in Swing need to be updated or figuratively shot with a bit of common sense.
Please don't keep building new things in Swing. Let that gruesome part of Java just die. It deserves to rest. Use JavaFX instead. It's newer, cleaner and the "new" intended way to build thick Java apps.