Creating Geometric Objects from a list of parameters in a file and comparing their areas.

[Charles Cole]

I'm trying to take the information from this file with the parameters SHAPE, RADIUS, HEIGHT, WIDTH, COLOR, FILLED. I'm struggling to figure out how to assign the default parameters to the newly created objects if they do not have all of the parameters listed in the file (the last 3 objects in the file). After that I am also confused on how to compare the areas of each object so I can write a method getLargestObject() that will let me print out the object's information with the largest area and its position in the ArrayList.

Right now when it runs through the program it gives an ArrayListOutOfBoundsException error when it reaches the 4th line without the filled parameter and it will do the same for the last 2 objects.

Data File:

CIRCLE, 1, blue, true
RECTANGLE, 1, 2, blue, true
RECTANGLE, 10, 2, red, true
CIRCLE, 2, green
RECTANGLE
CIRCLE

Driver:

package csu.cole;

import java.io.File;
import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.Scanner;

public class Driver {
	public static void main(String[] args) throws FileNotFoundException {
		Scanner input = new Scanner(new File(
				"C:/Users/Charles/Desktop/GeometricObjectsData.txt"));

		ArrayList<GeometricObject> list = new ArrayList<GeometricObject>();

		while (input.hasNextLine()) {
			String line = input.nextLine();
			String[] tokens = line.split(", ");
			if (tokens[0].equals("CIRCLE")) {
				Circle c = new Circle();
				float radius = Float.parseFloat(tokens[1]);
				c.setRadius(radius);
				String color = String.valueOf(tokens[2]);
				c.setColor(color);
				Boolean filled = Boolean.valueOf(tokens[3]);
				c.setFilled(filled);
				c.getArea();
				list.add(c);
				System.out.println(c.toString());
			} else if (tokens[0].equals("RECTANGLE")) {
				Rectangle r = new Rectangle();
				float height = Integer.parseInt(tokens[1]);
				r.setHeight(height);
				float width = Integer.parseInt(tokens[2]);
				r.setWidth(width);
				String color = String.valueOf(tokens[3]);
				r.setColor(color);
				Boolean filled = Boolean.valueOf(tokens[4]);
				r.setFilled(filled);
				r.getArea();
				list.add(r);
				System.out.println(r.toString());
			}

		}
		
	}

}

GeometricObject:

package csu.cole;

public abstract class GeometricObject {
		
		//class variables
		private String color;
		private boolean filled;
		
		//constructors
		public GeometricObject() {
			super();
			color = "white";
			filled = false;
		}

		public GeometricObject(String color, boolean filled) {
			super();
			this.color = color;
			this.filled = filled;
		}
		
		//mutators
		public String getColor() {
			return color;
		}

		public void setColor(String color) {
			this.color = color;
		}

		public boolean isFilled() {
			return filled;
		}

		public void setFilled(boolean filled) {
			this.filled = filled;
		}
		
		//user-defined methods
		public abstract double getArea();
		
		public abstract void getLargestObject();
		
		public int compareTo(GeometricObject other) {
			if(this.getArea() == other.getArea())
				return 0;
			else if (this.getArea() > other.getArea())
				return 1;
			else
				return -1;
		}
		@Override
		public String toString() {
			return super.toString() + " \tColor=" + this.getColor() + " \tFilled=" + this.isFilled();
			
		
	}

}

Circle:

package csu.cole;

import csu.cole.GeometricObject;

public class Circle extends GeometricObject {
	//class variables
	private double radius;
	
	//constructors
	public Circle() {
		super();
		radius = 1;
	}
	
	public Circle(double radius, String color, boolean filled) {
		super(color, filled);
		this.radius = radius;
	}
	
	//mutators
	public double getRadius() {
		return radius;
	}

	public void setRadius(double radius) {
		this.radius = radius;
	}
	
	//user-defined methods
	@Override
	public double getArea() {
		//area of a circle
		return (radius * radius * Math.PI);
	}

	@Override
	public String toString() {
		return super.toString() + "\nCircle: Radius=" + this.getRadius() + "\tArea= " + this.getArea();
	}
	
}

Rectangle:

package csu.cole;

import csu.cole.GeometricObject;

public class Rectangle extends GeometricObject {
	//class variables
	private double height;
	private double width;

	//constructors
	public Rectangle() {
		super();
		height = 1;
		width = 1;
	}
	public Rectangle(double height, double width, String color, boolean filled) {
		super(color,filled);
		this.height = height;
		this.width = width;
	}
	
	//mutators
	public double getHeight() {
		return height;
	}
	public void setHeight(double height) {
		this.height = height;
	}
	public double getWidth() {
		return width;
	}
	public void setWidth(double width) {
		this.width = width;
	}
	
	//user-defined methods
	@Override
	public double getArea() {
		return (height * width);
	}
	
	@Override
	public String toString() {
		return super.toString() + "\nRectangle: Height=" + this.height + "\tWidth=" + this.width + "\tArea= " + this.getArea();
	}

}

[Joel McNary]

OK, let's start by looking at the first problem...array index out of bounds. You are getting tokens[1], [2], etc..., but those values do not exist. So what you want to do is check to see if those values exist first and then read them. If they don't exist...what should your application do? What do you (or your instructor) want the application to do in that case (typical answers are "Use a default value" and/or "throw an exception")? In any case, can you translate that logic to code? If the value exists, then read it, else do something else. (Hint: tokens.length will be useful)

[Charles Cole]

Joel McNary wrote:OK, let's start by looking at the first problem...array index out of bounds. You are getting tokens[1], [2], etc..., but those values do not exist. So what you want to do is check to see if those values exist first and then read them. If they don't exist...what should your application do? What do you (or your instructor) want the application to do in that case (typical answers are "Use a default value" and/or "throw an exception")? In any case, can you translate that logic to code? If the value exists, then read it, else do something else. (Hint: tokens.length will be useful)

Okay I changed it to something like this:

		while (input.hasNextLine()) {
			String line = input.nextLine();
			String[] tokens = line.split(", ");
			if (tokens[0].equals("CIRCLE")) {
				Circle c = new Circle();
				if (tokens.length == 4) {
					float radius = Float.parseFloat(tokens[1]);
					c.setRadius(radius);
					String color = String.valueOf(tokens[2]);
					c.setColor(color);
					Boolean filled = Boolean.valueOf(tokens[3]);
					c.setFilled(filled);
					c.getArea();
					list.add(c);
					System.out.println(c.toString());
				} else if (tokens.length == 3) {
					float radius = Float.parseFloat(tokens[1]);
					c.setRadius(radius);
					String color = String.valueOf(tokens[2]);
					c.setColor(color);
					Boolean filled = false;
					c.setFilled(filled);
					c.getArea();
					list.add(c);
					System.out.println(c.toString());
				} else if (tokens.length == 1) {
					float radius = 1;
					c.setRadius(radius);
					String color = "white";
					c.setColor(color);
					Boolean filled = false;
					c.setFilled(filled);
					c.getArea();
					list.add(c);
					System.out.println(c.toString());
				}
			else if(tokens[0].equals("RECTANGLE")) {
				Rectangle r = new Rectangle();
				float height = Integer.parseInt(tokens[1]);
				r.setHeight(height);
				float width = Integer.parseInt(tokens[2]);
				r.setWidth(width);
				String color = String.valueOf(tokens[3]);
				r.setColor(color);
				Boolean filled = Boolean.valueOf(tokens[4]);
				r.setFilled(filled);
				r.getArea();
				list.add(r);
				System.out.println(r.toString());
				}

			}

I didn't finish the rectangle one, but I'm not sure how to call upon the other classes to assign the default values rather then assigning them again in the main class. Also, I think I nested my if statements incorrectly because now the program only shows the output of the circle objects instead of both the circle and rectangle objects.

What did I do wrong where it doesn't read the rectangle objects anymore? And is there anyway to consolidate that or do I need to write that many if, if-else statements?

[Piet Souris]

What about giving each GeometricObject a constructor that takes
the complete inputline? It would then be the class' responsibility
to deal with the given input, throwing an error if it cannot deal
with that input? It would also be the responsibility of such
a class to output the data.

Greetz,
Piet

[Joel McNary]

I agree with Piet, and that would have been a nit-pick further down the road once we got the logic right. You are on the right track; I might suggest something like, where the all-caps values are constants you have defined somewhere:

float radius = DEFAULT_RADIUS_VALUE;
String color = DEFAULT_COLOR_VALUE;
boolean filled = DEFAULT_FILLED_VALUE;
if(tokens.length > 1) {
    radius =  Float.parseFloat(tokens[1]);
}
if(tokens.length > 2) {
...etc...
...continue for the other parameters....

And to Piet's point, I would have this code in the constructor for the Circle class (taking the array of tokens as a parameter), making it the Circle's responsibility to parse the tokens, not the Driver's.

Now, since we are thinking about responsibility, let's think about the second part of your original question: who's responsibility should it be to find the largest object? Should a GeometricObejct be responsible for that? If so, how do you decide which one? If not, do you have another class that can take that responsibility? (OK, I'll admit I'm leading you by the nose here...hint: you have a Driver class...but these are questions to ask yourself when coding larger applications, so it is useful to ask them now in the smaller ones so you get into the habit of thinking about good design.)

[Joel McNary]

And, yes, you are missing/misplaced a close-brace; your if statements are nested wrong.

[Campbell Ritchie]

… and welcome to the Ranch

[Joanne Neal]

Joel McNary wrote:I agree with Piet

I disagree with Piet (and Joel).
By passing the text file line to the class constructor you are introducing coupling between the GeometricShape class (and it subclasses) and the format of the data file.
The GeometricShape class should know nothing about where its data has come from.

[Joanne Neal]

Charles Cole wrote:I'm not sure how to call upon the other classes to assign the default values rather then assigning them again in the main class.

You don't need to. It's already done for you in the the no args constructors. Get rid of all the code in your driver class that sets default values and you should find they are set anyway.

Also by changing the comparison operator (and possibly the order) of your if statements you can avoid having repeated code in your if statements.
You also seems to have missed out what happens if there are 2 items in the data. I know your current data doesn't need it but other data sets might.

[Piet Souris]

If the GeometricObject outputs its data in text form, and getting it back
in the same form then the whole thing is the responsibility of the
GO. What coupling would there be?

Greetz,
Piet

[Joanne Neal]

Piet Souris wrote:If the GeometricObject outputs its data in text form

Where does the GeometricObject class do that ? There's no indication of where the data is created.
What happens in the future if you want to keep the data in a database or an xml file ? Are you going to rewrite the GeometricObject class (and all its subclasses) or are you going to add a new parsing method to a GeometricObjectBuilder class that parses the new data source and then creates the appropriate class.

[Piet Souris]

Gee, one thing at a time, please.

The question here was how to read data for various classes from some data file.
Problem was that not all possible classes had an equal amount of data, and there
was the problem of default data. Handling all these problems was better left to
all the classes themselves. That's what I suggested, no more, no less.

If you foresee other construction mechanisms, then that's just a matter of
putting other data saving and data reading mechanisms in the classes. You can
go as far with this as you like. But it sounds a bit heavy for what OP is asking.

[Joanne Neal]

I guess we'll have to agree to disagree. The way that the OP has his shape classes at the moment is totally independent of the data format and already handles default data values (although he hadn't realised this). Suggesting he adds a constructor to handle a specific data format is a step backwards IMHO.

[Charles Cole]

package csu.cole;

import java.io.File;
import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;

public class Driver {
	public static void main(String[] args) throws FileNotFoundException {
		Scanner input = new Scanner(new File(
				"C:/Users/Charles/Desktop/GeometricObjectsData.txt"));

		ArrayList<GeometricObject> list = new ArrayList<GeometricObject>();

		while (input.hasNextLine()) {
			String line = input.nextLine();
			String[] tokens = line.split(", ");
			if (tokens[0].equals("CIRCLE")) {
				Circle c = new Circle();
				if (tokens.length == 4) {
					float radius = Float.parseFloat(tokens[1]);
					c.setRadius(radius);
					String color = String.valueOf(tokens[2]);
					c.setColor(color);
					Boolean filled = Boolean.valueOf(tokens[3]);
					c.setFilled(filled);
					c.getArea();
					list.add(c);
					System.out.println(c.toString());
				} else if (tokens.length == 3) {
					float radius = Float.parseFloat(tokens[1]);
					c.setRadius(radius);
					String color = String.valueOf(tokens[2]);
					c.setColor(color);
					Boolean filled = false;
					c.setFilled(filled);
					c.getArea();
					list.add(c);
					System.out.println(c.toString());
				} else if (tokens.length == 1) {
					String color = "white";
					c.setColor(color);
					Boolean filled = false;
					c.setFilled(filled);
					c.getArea();
					list.add(c);
					System.out.println(c.toString());
				}
			} else if (tokens[0].equals("RECTANGLE")) {
				Rectangle r = new Rectangle();
				if (tokens.length == 5) {
					float height = Integer.parseInt(tokens[1]);
					r.setHeight(height);
					float width = Integer.parseInt(tokens[2]);
					r.setWidth(width);
					String color = String.valueOf(tokens[3]);
					r.setColor(color);
					Boolean filled = Boolean.valueOf(tokens[4]);
					r.setFilled(filled);
					r.getArea();
					list.add(r);
					System.out.println(r.toString());
				} else if (tokens.length == 1) {
					String color = "white";
					r.setColor(color);
					Boolean filled = false;
					r.setFilled(filled);
					r.getArea();
					list.add(r);
					System.out.println(r.toString());
				}
			}
			}

		}
		public int getLargestObject() {
			int max = Integer.MIN_VALUE;
			for (int = 0; i < list.size(); i++){
				if (list.get(i) > max){
					max = list.get(i);
				}
			}
			return max;
	}
}

So I got the first part of the problem working, but I'm not sure how to get the second part. I need to write a method call getLargestObject() that returns the position in the ArrayList<GeometricObjects> of the object with the largest area and print the position and contents of the largest object. Now I think I need to create a new array list that stores only the areas and then use the for loop I created at the end to get the largest area and then how would I return the object's position in the ArrayList<GeometricObjects> which has the largest area.

The object with the largest area will be the 3rd object being a Rectangle with an area of 20.

[Junilu Lacar]

OP should read through this thread carefully and note the different arguments for/against one approach vs another. There are good points about design choices being raised. So now, I'll throw in my (hopefully unifying) opinion.

I agree that the objects themselves, not the driver, should contain the logic on how to handle data and when to use which constructor. This is OOP, objects should be asked to do something, not told.

I agree that passing the input line to the objects and have them parse it is probably not a good idea. You need to separate responsibilities and that's probably pushing the edge.

When parameter lists start getting too long, one strategy to get a handle on complexity is to introduce another object that encapsulates those parameters. If you think about some frameworks you've used (Quartz scheduler comes to mind), you'll notice that they have a concept of "Context" that gets passed around to various objects to use for things like initializing their own state. If you look at DAOs, you'll see that many deal with one object, the domain object, but can ask the domain object for any number of its attributes to persist in storage. We can borrow these strategies and apply them to this problem.

You can introduce another kind of object to hold all the initialization parameters. Have some way of asking that object if it has an initial value. You might even tell that object "Give me the value of Blah. If you don't have a value for Blah, use this value as the default". This strategy is also fairly common in the wild.

So here's the "conversation" that I imagine would go on in the program:

Driver: (reads a line of input)

Driver, to a new GeometryParams: "Please initialize yourself with this input data"

GeometryParams: (takes data and initializes itself with it)

Driver, to the GeometryParams: "What kind of shape is it?"

GeometryParams: "It's a [CIRCLE, RECTANGLE, whatever]"

Driver: "Ok" ... (creates [CIRCLE, RECTANGLE, whatever])

Driver, to new CIRCLE, RECTANGLE, whatever: "Please work with this GeometryParams to finish up your initialization."

CIRCLE, RECTANGLE, whatever, to GeometryParams: "Please give me the value of Blah. If you don't have one, just give me BlahBlah as a default."

and so on.

===

Advantages:
1. Only GeometryParams has to care about exact format of the input data. From what I can tell, the data indices are fairly consistent. First item appears to be always the shape type. Other variations can be easily handled from there.
2. Parameter list involves only GeometryParams. If input data changes in the future, the parameter list doesn't have to change.
3. If input data does change, then changes that need to be made are fairly localized. High-level concepts and interactions will remain the same.

[Junilu Lacar]

And of course when you look at the code long enough, you'll probably start thinking about a Factory but I don't think you have to go that far. Don't try to build in too much flexibility until you know you need it. Just do what it takes to make it good enough for now and easy enough to change later.

[Junilu Lacar]

There's also a lot of duplication in the logic. Note the following:

Given array x,

if x.length > 0 then you are assured that x[0] exists
if x.length > 1 then you are assured that x[1] exists
if x.length > 2 then you are assured that x[2] exists
if x.length > 3 then you are assured that x[3] exists
and so on

You can use this to eliminate a lot of duplicated code. Remember DRY => Don't Repeat Yourself!