Creating an abstract Shape class
I am creating a class of shapes which of course will be abstract. I derived from this 3 classes; traingle, circle and rectangle. I am wanting to have the Shape constructor parameters set into a variable list array. Like this:
protecteddouble [] sideLength;
protectedint numSides;
public Shape (double...sideLength)
{
this.sideLength = sideLength;
this.numSides = numSides;
}
i will jump to say my Triangleclass:
publicclass Triangleextends Shape
publicclass Traingle (double side1,double side2,double side3)
{
this.sideLength[0];
this.sideLength[1];
this.sideLength[2];
}
is that a correct way to initailize the triangle constructor? Thanks
[1283 byte] By [
THEjima] at [2007-11-27 11:44:53]
Doesn't look right to me. Nowhere do you allocate memory for the sideLength array.
%
> is that a correct way to initailize the triangle
> constructor? Thanks
No. Constructors are not inherited, you know. Your constructors as written don't work. In the Shape class, you assign the instance field, this.numSides to itself since you don't pass a numSides parameter. perhaps you meant something like this.numSides = sideLength.length;
In the triangle class, you have not initilized any array called sideLength, and even if you did, just enumerating the array items does not assign them anything. I can sort of see what you want to do, you want to have the abstract class hold the array of sides. If you ever want to use a parent class's constructor, you must call it explicitly with super(). or super(parameters), and this must be the first line of the child's constructor.
public abstract class Shape {
private double [] sideLength;
//protected int numSides; redundant -- use sideLength.length
protected Shape(double... sideLength) {
this.sideLength = sideLength;
}
}
public class Triangle extends Shape {
public Triangle (double side1, double side2, double side3) {
super(side1, side2, side3);
}
}
Sorry, I'm a dope. I haven't done enough with variable parameter lists. More in a bit.
%
What about the Groovy version, BDLH? ;o)
~
> What about the Groovy version, BDLH? ;o)
I haven't been groovy since the seventies. It was the afro perm, mostly.
Yea, i forgot to include the super() reference. I thank you all for the help, i'm going to tinker around with it and see what i can do.
Thanks
Like this:
package cruft.shape;
/**
* Shape
* User: Michael
* Date: Jul 25, 2007
* Time: 5:44:38 PM
*/
public abstract class Shape
{
protected double [] sideLengths;
protected int numSides;
public Shape(double... sideLengths)
{
this.sideLengths = sideLengths;
this.numSides = sideLengths.length;
if (sideLengths.length == 0)
throw new IllegalArgumentException("Shape lengths must be positive");
for (double sideLength : sideLengths)
{
if (sideLength <= 0.0)
{
throw new IllegalArgumentException("Triangle side lengths must be positive");
}
}
}
public abstract double area();
public abstract double perimeter();
}
And this:
package cruft.shape;
/**
* Triangle
* User: Michael
* Date: Jul 25, 2007
* Time: 5:50:08 PM
*/
public class Triangle extends Shape
{
public static final int DEFAULT_NUM_SIDES = 3;
public static void main(String[] args)
{
double a = ((args.length > 0) ? Double.valueOf(args[0]) : 1.0);
double b = ((args.length > 1) ? Double.valueOf(args[1]) : 1.0);
double c = ((args.length > 2) ? Double.valueOf(args[2]) : 1.0);
Triangle tria = new Triangle(a, b, c);
System.out.println("perimeter: " + tria.perimeter());
System.out.println("area: " + tria.area());
}
public Triangle(double... sideLengths)
{
super(sideLengths);
if (this.numSides != DEFAULT_NUM_SIDES)
{
throw new IllegalArgumentException("Triangle must have " + DEFAULT_NUM_SIDES + " sides");
}
}
public double area()
{
double s = perimeter()/2.0;
double area = s;
for (double sideLength : sideLengths)
{
area *= (s-sideLength);
}
return Math.sqrt(area);
}
public double perimeter()
{
double perimeter = 0.0;
for (double sideLength : sideLengths)
{
perimeter += sideLength;
}
return perimeter;
}
}
%
Thanks for the tips duffymo!
So, then when i create my Rectangle class it will be something to this effect:
public class Rectangle extends Shape
{
public Rectangle (double side1, double side2)
{
super( side1, side2);
}
public double calcArea();
{
return side1 * side2;
}
}
then i will override my calcArea and calcPerimeter methods from Shape class. Looks to me like i can't just use say side1 and side2 to calculate the area and perimeter, if the values eneterd from a main driver class are as follows say:
Shape r = new Rectangle(2.0, 5.0);
System.out.print(r.calcArea()):
The values for each shape will be enetred into an array, someone mentioned to me about using
System.arraycopy()
> So, then when i create my Rectangle class it will be something to this effect:
Every rectangle I know of has four sides, not two, so you'll want to look at your call to the superclass constructor.
~
> public abstract class Shape
> {
>protected double [] sideLengths;
> protected int numSides;
>
>public Shape(double... sideLengths)
> {
>this.sideLengths = sideLengths;
> this.numSides = sideLengths.length;
>
>if (sideLengths.length == 0)
> throw new IllegalArgumentException("Shape lengths
> must be positive");
>
>for (double sideLength : sideLengths)
> {
> if (sideLength <= 0.0)
> {
> throw new
> IllegalArgumentException("Triangle side lengths must
> be positive");
> }
>
>}
>public abstract double area();
> public abstract double perimeter();
Just to nitpick, the Shape class shouldn't throw an Exception with a message referring to a subclass (Triangle, in this case), but I think that's probably an inadvertent inclusion.
~
You caught me. Bad cut & paste job. I originally had that check in the Triangle class before I realized that it rightly belonged in Shape. I didn't realize that I should have changed the exception message until after I posted.
If I were a real professional, I would have externalized my exception messages so I could do I18N, but I was being a lazy ******. 8)
%
Yes, it does have 4 sides, i'm just using two sides for this class to calculate area and perimeter.
Assuming that a Rectangle has two pairs of equal length sides, so you only need two lengths.
p = 2*(b+h)
a = b*h
%
If you know what Gaussian quadrature is and contour integration, it'll be easy to generalize this for an n-sided polygon.
%
> Yes, it does have 4 sides, i'm just using two sides
> for this class to calculate area and perimeter.
Think about the design of your Shape class, then. A Triangle has three sides, a Rectangle has two sides, and a Pentagon has...?
~
> Assuming that a Rectangle has two pairs of equal
> length sides, so you only need two lengths.
Why bother with an inherited array of sides, then? I presumed from BDLH's earlier post that there is some interest in using the array length to determine the number of sides.
> If you know what Gaussian quadrature is and contour integration...
You can stop right there. :o)
~
> > Assuming that a Rectangle has two pairs of
> equal
> > length sides, so you only need two lengths.
>
> Why bother with an inherited array of sides, then? I
> presumed from BDLH's earlier post that there is some
> interest in using the array length to determine the
> number of sides.
Good point.
I would agree that a four-sided Rectangle would be preferred. It's more general, because it still works for a Quadrilateral with four sides that may all be different lengths.
>
> > If you know what Gaussian quadrature is and contour
> integration...
>
> You can stop right there. :o)
Just trying to be general. For straight-sided polygons in 2D space, it's particularly easy. In that case, it'd be better to have a variable number of (x, y) points as the input. Assume linear shape functions and voila!
%
> It's more general, because it still works for a Quadrilateral with four sides that may all be different lengths.
I was thinking along the simple lines of something like...public Rectangle(double w, double h) {
super(w, h, w, h);
}
Note that the perimeter calculation would then be the same as Triangle's, and you've got a good candidate for refactoring for DRY purposes. :o)
> > You can stop right there. :o)
>
> Just trying to be general.
I know, but you've charged into territory *way* out of my league. I'm afraid I'll have to leave all that to you and the rest of the math gurus around here. I'm but a neophyte. :o)
~
If the Shape constructor takes an argument an array of lengths, be aware that the caller will be able to change these lengths at will even though the array is protected or private. Perhaps it would be better to copy the array argument.
> Perhaps it would be better to copy the array argument.
Definitely.
Duffymo, we're just going to nitpick your academic example to pieces. Better get started on that i18n version that you'll be exposing as a web service... ;o)
~
I was thinking about using the
System.arraycopy()
maybe passing the values from sideLength to this.sideLength
I'm using the array technique so i can later sort an array of objects using insertion sort.
public abstract class Shape {
protected double [] sideLengths;
public Shape(double... sideLengths) {
this.sideLengths = sideLengths.clone();
if (this.sideLengths.length == 0) {
throw new IllegalArgumentException("Shape lengths must be positive");
}
for (double sideLength : this.sideLengths) {
if (sideLength <= 0.0) {
throw new IllegalArgumentException("Side lengths must be positive");
}
}
}
public abstract double area();
public abstract double perimeter();
}
~
> I was thinking about using the
> System.arraycopy()
> maybe passing the values from sideLength to this.sideLength
Yes, somesuch is needed. Since you mentioned using var args in your OP, I thought it was worth mentioning a possible problem.
> If the Shape constructor takes an argument an
> array of lengths, be aware that the caller
> will be able to change these lengths at will even
> though the array is protected or private. Perhaps it
> would be better to copy the array argument.
That's a GREAT point. No privacy at all the way I did it.
%
Thinking about it a bit more, the idea of passing in side lengths only works for the simplest of polygons.The area calculation screws everything up.
Imagine a Rectangle with two pairs of equal length sides and four right angles. Easy to calculate the area then: it's just the product of the lengths from each equal pair.
Now imagine that Rectangle with the opposite pairs of angles changing - one pair decreasing from a right angle, the other pair increasing in angle. The lengths of the sides haven't changed, but you can see that the area will decrease to zero when the Rectangle collapses completely.
So the sides aren't enough. You have to declare general polygons in terms of arrays of Point2Ds.
%
Alrght well i worked on my Shape class last night, here is what i have so far. It seems my Circle calculation is not correct.
I'm not sure exactly how to incorporate the System.arraycopy feature so its unfinished at the moment. Oh and my boolean expression is not completed yet either, but everything else runs ok it seems.
public abstract class Shape
{
protected int numSides;
protected double[] sideLength;
public static final double PI = 3.1415926535897932384626433832795;
public Shape(double...sideLength)
{
this.sideLength = sideLength;
this.numSides = numSides;
System.arraycopy(sideLength, 0, this.sideLength, 0, 0);
}
public final int getNumSides()
{
return sideLength.length;
}
public final double[] getSideLength()
{
return sideLength;
}
public abstract double calcArea();
public abstract double calcPerimeter();
public boolean equals (Object obj)
{
Shape s = ((Triangle)obj);
if (this.numSides != s.numSides)
return false;
else
return true;
}
public static final void insertionSort (Shape[] list)
{
int min;
Shape key;
for(int index = 0; index < list.length; index ++)
{
key = list[index];
int position = index;
while (position > 0 && key.calcArea()< (list[position-1]).calcArea())
{
list[position] = list[position-1];
position--;
}
list[position] = key;
}
}
}
public Triangle(double side1, double side2, double side3)
{
super(side1, side2, side3);
}
public double calcPerimeter()
{
double perimeter;
return perimeter = (sideLength[0] + sideLength[1] + sideLength[2]) / 2 ;
}
public double calcArea()
{
double perimeter;
perimeter = (sideLength[0] + sideLength[1] + sideLength[2]) / 2 ;
return Math.sqrt(perimeter*(perimeter-sideLength[0])*
(perimeter-sideLength[1])*(perimeter-sideLength[2]));
}
public String toString()
{
return "Triangle with sides: " + sideLength[0] + "," + sideLength[1] +
"," + sideLength[2];
}
}
public class Rectangle extends Shape
{
public Rectangle(double side1, double side2)
{
super (side1, side2) ;
}
public double calcArea()
{
return sideLength[0] * sideLength[1];
}
public double calcPerimeter()
{
return 2 * (sideLength[0] + sideLength[1]);
}
public String toString()
{
return "Rectangle with sides:" + sideLength[0] +
"," + sideLength[1] ;
}
}
public class Circle extends Shape
{
private double diameter;
public Circle(double radius)
{
super(radius);
}
public double calcArea()
{
return PI * sideLength[0] * sideLength[0];
}
private double calcDiameter()
{
return 2 * sideLength[0];
}
public double calcPerimeter()
{
double circumference;
circumference = 2 * PI * sideLength[0];
return circumference;
}
public String toString()
{
return "Circle with radius: " + sideLength[0];
}
}
public class TestShape
{
public static void main (String []args)
{
Shape a = new Circle(25);
System.out.print(a.calcArea());
System.out.print(a.calcPerimeter());
System.out.println(a.getNumSides());
Shape b = new Triangle(5,6,9);
System.out.println(b.calcArea());
System.out.println(b.calcPerimeter());
Shape r = new Rectangle (2,2);
System.out.print(r);
System.out.println(r.calcArea());
System.out.println(r.calcPerimeter());
System.out.println(r.getNumSides());
}
}
Adding a static PI to the Shape makes no sense whatsoever. Use Math.PI.
%
> > If the Shape constructor takes an argument an
> > array of lengths, be aware that the caller
> > will be able to change these lengths at will even
> > though the array is protected or private. Perhaps
> it
> > would be better to copy the array argument.
>
> That's a GREAT point. No privacy at all the way I
> did it.
>
> %
I had a question about the copying. The argument isn't an array or List, it's a bunch of primitives. Under the covers those primitives are turned into an array or List, but it almost feels like it's local to the method. Nobody outside the method will have access to that array or List reference, so nobody can change it. No need to copy or clone the argument. True?
%
> I had a question about the copying. The argument
> isn't an array or List, it's a bunch of primitives.
Varargs is an array.
~
Well, i really want to use the insertion sort to sort an array of shapes.
However i do see what you are saying
This is my output from running the program, i'm not sure how that large number on the first line came about, any idea? My calculations for triangle and rectangle aren't showing up either
1963.4954084936207157.079632679489661
Circle with radius: 25.0
14.142135623730951
10.0
3
Triangle with sides: 5.0,6.0,9.0
Rectangle with sides:2.0,2.04.0
8.0
2
> > I had a question about the copying. The
> argument
> > isn't an array or List, it's a bunch of
> primitives.
>
> Varargs is an array.
>
> ~
Yes, but who holds the reference to it?
If I write:
Triangle t = new Triangle(1.0, 2.0, 3.0);
the arguments are three primitives. The danger to privacy would come if the caller had a reference to the array and could change its values. In this case it looks to me like the array reference is internal to the method when it's compiled. No?
I can easily see where this is a problem:
List<Double> sides = new ArrayList<Double>();
sides.add(1.0);
sides.add(2.0);
sides.add(3.0);
// caller has a reference to the private data member. change the values of the reference, change private state.
Triangle t = new Triangle(sides);
I don't see how I can do that with varargs.
%
You should either make Shape implement Comparable OR write a ShapeComparator for sorting.Nobody should be writing sort routines. Collections.sort will do it for you.
%
Ok i have two abstract methods in the parent class Shape:
public abstract double calcArea();
public abstract double calcPerimeter();
Which i override in my child classes, Triangl, Rectangle, and Circle. I want a method ONLY in the circle class called
public double calcDiameter();
from my driver class it keeps saying can not find method in Shape class. I can't put it in Shape because not every shape has a diameter, what is the fix for this scenario?
> I want a method ONLY in the
> circle class called
> > public double calcDiameter();
>
>
> from my driver class it keeps saying can not find
> method in Shape class. I can't put it in Shape
> because not every shape has a diameter, what is the
> fix for this scenario?
Shape shape = ...
if (shape instanceof Circle) {
Circle circle = (Circle) shape;
double diameter = circle.getDiameter();
...
}
By the way, have you looked at java.awt.Shape to see what they thought
a shape should look like, for comparison?
http://java.sun.com/javase/6/docs/api/java/awt/Shape.html
Horses for courses...
> Which i override in my child classes, Triangl,
> Rectangle, and Circle. I want a method ONLY in the
> circle class called
> > public double calcDiameter();
>
> from my driver class it keeps saying can not find
> method in Shape class. I can't put it in Shape
> because not every shape has a diameter, what is the
> fix for this scenario?
Exactly. It's called the slicing problem.
It's a bad design, because now you have to know what kind of Shape you're dealing with and cast to get that method.
Why do you think you need calcDiameter? What's the point for a user of Shapes?
My recommendation would be to eliminate that method from Circle. Users of Shape care about two things: perimeter and area calculations for 2D planar shapes. That's it.
%
PS - From your previous post it appears that you have some output problems. It's not enough to write code and have it print values. Get out a calculator, plug in some numbers, and be certain that the calculations are correct. Give a few test cases, including some edge cases, to make sure that it behaves correctly in all scenarios. What if you give negative side lengths? What if a size is zero?
Learn something about JUnit: http://www.junit.org
Capture your tests in code. Better than main methods and command line args.
%
> Yes, but who holds the reference to it?
Ah. Now I get what you're saying. I think the concern is that it's possible to pass an array of doubles to a varargs method, thereby retaining a reference to the array outside the object. Therefore, it's still "safer" to clone/copy.
~
Should even circle be a subtype of this shape class? He seems to be defining his shape objects by side lengths and number of sides. Circle seems to be a different animal completely.
I would have made Shape an interface. There's too many possibilities
for implementation approaches to fit that into a class.
> > Yes, but who holds the reference to it?
>
> Ah. Now I get what you're saying. I think the concern
> is that it's possible to pass an array of doubles to
> a varargs method, thereby retaining a reference to
> the array outside the object.
I just tried it in my sample app, and now I see your point. Instead of passing three doubles to Triangle I made a double array. Then I changed one of the values in array held onto by the client, and hijinks ensured.
> Therefore, it's still
> "safer" to clone/copy.
>
> ~
Agreed. Good point.
Thanks for the lesson. I've done nothing with varargs until today.
%
