patternjavaMinor
Drawing the appropriate shapes onto JFrames
Viewed 0 times
theappropriateontodrawingshapesjframes
Problem
To write a Java class that reads through a text file of drawing commands and draws the appropriate shapes onto JFrames, I have input instructions as follows:
And the following is how I did it:
```
import java.awt.Color;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Rectangle;
import java.awt.Shape;
import java.awt.geom.Ellipse2D;
import java.awt.geom.Line2D;
import java.io.File;
import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.Scanner;
import javax.swing.JComponent;
import javax.swing.JFrame;
public class Driver {
private static Frame frame;
private static Component component;
private static Color currentColor = Color.BLACK;
private static Shape currentShape;
private static int frameNo = 0;
private static int lineNo = 0;
private static ArrayList frame
FRAME width height // sets up a new frame with given width and height (both integers)
COLOR red green blue // sets the current “pen color” to the color with the given rgb components.
RECTANGE x y width height // draws a rectangle with upper left corner at x,y and given width and height (all //
given in integers)
CIRCLE x y radius // draws a circle centered at x,y with given radius (as doubles)
ELLIPSE x y xradius yradius // draws an ellipse centered at x,y with semi-radii xradius and yradius (Note:
// these parameters are slightly different than that in Ellipse2D.Double’s
// constructor.
LINE x1 y1 x2 y2 // draws a line from x1,y1 to x2,y2 (as doubles)
A sample command paint_instructions.txt file is as follows:
FRAME 200 100 // open a frame, note: parser must ignore any comments
COLOR 255 0 0 // set color to red
RECTANGLE 20 30 40 20 // draw a red rectangle
COLOR 128 128 128 // set color to gray
CIRCLE 100 50 25 // draw a gray circle
FRAME 100 100 // open a second frame
COLOR 0 0 255 // set color to blue
ELLIPSE 50 50 30 20 // draw a blue ellipse
COLOR 0 255 0 // set color to green
LINE 10 20 90 80 // draw a green lineAnd the following is how I did it:
```
import java.awt.Color;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Rectangle;
import java.awt.Shape;
import java.awt.geom.Ellipse2D;
import java.awt.geom.Line2D;
import java.io.File;
import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.Scanner;
import javax.swing.JComponent;
import javax.swing.JFrame;
public class Driver {
private static Frame frame;
private static Component component;
private static Color currentColor = Color.BLACK;
private static Shape currentShape;
private static int frameNo = 0;
private static int lineNo = 0;
private static ArrayList frame
Solution
Overall this is pretty straightforward. I just have a few minor suggestions.
Error Handling
What happens when a file is invalid? For example, if it doesn't contain a "FRAME" line? If it just starts with, say, "CIRCLE", should an error be generated?
Don't Repeat Yourself
I see these 2 lines in every case statement after "COLOR":
Because it's for each shape, you could change the switch statement to something like:
And then
Also, your
Use Named Constants
In your
and then in the code you could do stuff like:
(My apologies if I have the syntax wrong - I can't remember Java's syntax for constants! Hopefully the idea is clear.)
Error Handling
What happens when a file is invalid? For example, if it doesn't contain a "FRAME" line? If it just starts with, say, "CIRCLE", should an error be generated?
Don't Repeat Yourself
I see these 2 lines in every case statement after "COLOR":
frames.get(frameNo-1).shapes.add(currentShape);
frames.get(frameNo-1).colors.add(currentColor);Because it's for each shape, you could change the switch statement to something like:
switch (a[0])
{
case "FRAME": // do frame stuff
case "COLOR": // do color stuff
default: processShape(a);
}And then
processShape() would look something like:void processShape(String[] a)
{
switch (a[0])
{
case "RECTANGLE": currentShape = new Rectangle(Integer.parseInt(a[1]),Integer.parseInt(a[2]),Integer.parseInt(a[3]),Integer.parseInt(a[4]));
case "ELLIPSE": // ...etc.
// ...Other shapes...
default: System.out.println("Input Instruction Not Recognized");
return;
}
frames.get(frameNo-1).shapes.add(currentShape);
frames.get(frameNo-1).colors.add(currentColor);
}Also, your
Frame and Component classes look really similar, though they inherit from different base classes. In draw() you create a new Component and copy the Frame's shapes and colors into it, then add it to the Frame. That seems like unnecessary copying of the data. Do both the Component and the Frame need that data? If so, can one access the other's copy of it? Use Named Constants
In your
case statements, you use several members of the array you parsed directly by index. It would be easier to read and understand if you had named constants for the indices. Something like this:const int FRAME_WIDTH_IDX = 1;
const int FRAME_HEIGHT_IDX = 2;
//...etc.and then in the code you could do stuff like:
frame.setSize(Integer.parseInt(a[FRAME_WIDTH_IDX]),Integer.parseInt(a[FRAME_HEIGHT_IDX]));(My apologies if I have the syntax wrong - I can't remember Java's syntax for constants! Hopefully the idea is clear.)
Code Snippets
frames.get(frameNo-1).shapes.add(currentShape);
frames.get(frameNo-1).colors.add(currentColor);switch (a[0])
{
case "FRAME": // do frame stuff
case "COLOR": // do color stuff
default: processShape(a);
}void processShape(String[] a)
{
switch (a[0])
{
case "RECTANGLE": currentShape = new Rectangle(Integer.parseInt(a[1]),Integer.parseInt(a[2]),Integer.parseInt(a[3]),Integer.parseInt(a[4]));
case "ELLIPSE": // ...etc.
// ...Other shapes...
default: System.out.println("Input Instruction Not Recognized");
return;
}
frames.get(frameNo-1).shapes.add(currentShape);
frames.get(frameNo-1).colors.add(currentColor);
}const int FRAME_WIDTH_IDX = 1;
const int FRAME_HEIGHT_IDX = 2;
//...etc.frame.setSize(Integer.parseInt(a[FRAME_WIDTH_IDX]),Integer.parseInt(a[FRAME_HEIGHT_IDX]));Context
StackExchange Code Review Q#85999, answer score: 2
Revisions (0)
No revisions yet.