Drawing the appropriate shapes onto JFrames

Question

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:

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 line

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> frames = new ArrayList<Frame>();


    public static void main(String[] args) throws FileNotFoundException {
        Scanner scan = new Scanner(System.in);
        System.out.println("Please enter the file name to read the drawing instructions from: ");
        String fileName = scan.next();
        scan.close();
        scan = new Scanner(new File(fileName));
        while(scan.hasNext()) {
            lineNo++;
            process(scan.nextLine());
        }
        scan.close();
        for (int i = 0; i < frames.size(); i++) {
            draw(frames.get(i));    
        }
    }


    public static void process(String s) {  
        String[] a = s.split(" ");

        // Strings can be used in switch statements in Java 7 and later versions
        try {
            switch(a[0]) {
                case "FRAME":       frameNo++;
                                    frame = new Frame("Frame-" + frameNo);
                                    frame.setSize(Integer.parseInt(a[1]),Integer.parseInt(a[2]));
                                    if (frameNo == 1) {
                                        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                                    } else {
                                        frame.setDefaultCloseOperation(JFrame.HIDE_ON_CLOSE);
                                    }
                                    frame.setVisible(true);
                                    frames.add(frame);
                                    break;

                case "COLOR":       currentColor = new Color(Integer.parseInt(a[1]),Integer.parseInt(a[2]),Integer.parseInt(a[3]));
                                    break;

                case "RECTANGLE":   currentShape = new Rectangle(Integer.parseInt(a[1]),Integer.parseInt(a[2]),Integer.parseInt(a[3]),Integer.parseInt(a[4]));
                                    frames.get(frameNo-1).shapes.add(currentShape);
                                    frames.get(frameNo-1).colors.add(currentColor);             
                                    break;

                case "ELLIPSE":     currentShape = new Ellipse2D.Double(Integer.parseInt(a[1]),Integer.parseInt(a[2]),Integer.parseInt(a[3]),Integer.parseInt(a[4]));
                                    frames.get(frameNo-1).shapes.add(currentShape);
                                    frames.get(frameNo-1).colors.add(currentColor);             
                                    break;  

                case "CIRCLE":      currentShape = new Ellipse2D.Double(Integer.parseInt(a[1]),Integer.parseInt(a[2]),Integer.parseInt(a[3]),Integer.parseInt(a[3]));
                                    frames.get(frameNo-1).shapes.add(currentShape);
                                    frames.get(frameNo-1).colors.add(currentColor);             
                                    break;

                case "LINE":        currentShape = new Line2D.Double(Integer.parseInt(a[1]),Integer.parseInt(a[2]),Integer.parseInt(a[3]),Integer.parseInt(a[4]));
                                    frames.get(frameNo-1).shapes.add(currentShape);
                                    frames.get(frameNo-1).colors.add(currentColor);             
                                    break;

                default: System.out.println("Input Instruction Not Recognized");    
            }
        } 

        catch (RuntimeException e) {
            e.printStackTrace();
            System.out.println("Invalid input instruction on line " + lineNo);
        }
    }


    public static void draw(Frame f) {
        Component component = new Component(f.shapes,f.colors);
        f.add(component);
    }
}

class Frame extends JFrame {
    public ArrayList<Shape> shapes;
    public ArrayList<Color> colors;
    public Frame(String s) {
        super(s);
        shapes = new ArrayList<Shape>();
        colors = new ArrayList<Color>();
    }
}

class Component extends JComponent {
    private ArrayList<Shape> shapes;
    private ArrayList<Color> colors;
    public Component(ArrayList<Shape> s, ArrayList<Color> c) {
        shapes = s;
        colors = c;
    }
    public void paintComponent(Graphics g) {
        super.paintComponent(g);
        Graphics2D g2 = (Graphics2D) g;
        for (int i = 0; i < shapes.size(); i++) {
            g2.setColor(colors.get(i));
            g2.draw(shapes.get(i));
        }
    }
}

Answer 1

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":

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.)