So I have this code:
https://gist.github.com/anonymous/0760d154f81064bf8493 (Sorry it's 5 classes so i couldn't put it here)
And I think I have implemented polymorphism in it to a decent extent. But is there anything I else I could do to make it even more polymorphic?
Also, in the main class, I made the local variables like so (shape,shape1,shape2, area, area1, area2) I feel like I shouldn't have had to do that because of inheritance or something. I'm not sure it just doesn't seem right.
Thanks
Polymorphism is not a tool that solves problems, it is a paradigm to follow if you want to.
That said, the question should be: what needs to be more polymorphic? Is there any gain from your change? Are there any requirements? What is the goal you are trying to archive?
Answering those questions will tell you what to change about your code, if it needs any change at all.
I would change MyShapes that way
import java.util.Scanner;
public class MyShapes
{
public static Scanner scan = new Scanner(System.in);
public static int Menu()
{
System.out.println("\nSelect a Shape or Exit: \n");
System.out.println("1. Square");
System.out.println("2. Circle");
System.out.println("3. Triangle");
System.out.println("4. Exit");
System.out.println("\nEnter choice:");
int option = scan.nextInt();
return option;
}// end menu
public static void main(String[] args)
{
int option = 0;
while (option != 4)
{
option = Menu();
Shape shape = null;
switch(option)
{
case 1:
shape = new Circle(scan.nextDouble());
break;
case 2:
Shape shape = new Square(scan.nextDouble());
break;
case 3:
shape = new Triangle(scan.nextDouble());
break;
case 4:
System.out.println("System closing");
System.out.println("-----------\n");
//System.exit(0); <-- not needed
break;
default:
System.out.println("Invalid option");
System.out.println("--------------\n");
}
printShape(shape); //check null needed.
}
}
private void printShape(Shape shape){
if(shape != null){
double boundaryLength1 = shape.getBoundaryLength();
double area1 = shape.getArea();
System.out.println("Boundary Length = " + Math.round(boundaryLength1));
System.out.println("Area = " + Math.round(area1));
}
}
}
And a really better option is to put that print inside each object, instead of MyShapes class.
Another thing, System.exit() is not a good practice, your code will end if Exit option is selected without needing to System.exit() it, just remove that line.
Factories
more advanced topic than inheritance are design patterns, when creating objects depending on a parameter it is prefered to use Factories.
public class ShapeFactory{
public static Shape createShape(int option){
//Your switch here.
}
}
and your MyShapes class will be changed to
import java.util.Scanner;
public class MyShapes
{
public static Scanner scan = new Scanner(System.in);
public static int Menu()
{
System.out.println("\nSelect a Shape or Exit: \n");
System.out.println("1. Square");
System.out.println("2. Circle");
System.out.println("3. Triangle");
System.out.println("4. Exit");
System.out.println("\nEnter choice:");
int option = scan.nextInt();
return option;
}// end menu
public static void main(String[] args)
{
int option = 0;
while (option != 4)
{
option = Menu();
Shape shape = ShapeFactory.createShape(option);
printShape(shape); //check null needed.
}
}
private void printShape(Shape shape){
if(shape != null){
double boundaryLength1 = shape.getBoundaryLength();
double area1 = shape.getArea();
System.out.println("Boundary Length = " + Math.round(boundaryLength1));
System.out.println("Area = " + Math.round(area1));
}
}
}
There's nothing wrong with the polymorphism itself. The way you're constructing the shapes is a bit weird though.
This would make a bit more sense:
if (option==CLOSE_OPTION) end();
Shape shape = getShape(option);
if (shape!=null) {
double boundaryLength = shape.getBoundaryLength();
double area = shape.getArea();
System.out.println("Boundary Length = " + Math.round(boundaryLength));
System.out.println("Area = " + Math.round(area));
}
...
private Shape getShape(int option){
switch(option) {
case 1:
return new Circle(scan.nextDouble());
case 2:
return new Square(scan.nextDouble());
case 3:
return new Triangle(scan.nextDouble());
default:
System.out.println("Invalid option");
System.out.println("--------------\n");
return null;
}
}
There's not really anything wrong with what you've done. Shape1/Shape2 etc are just private variables that you've used to store instances of 'Shape', which is normal enough...
I'm guessing this is a homework exercise, and you have demonstrated that you understand overriding methods from Superclasses, but if you wanted to go all out, you could make 'Shape' an interface with common method definitions, then Square/Triangle etc would have to implement the Shape interface. Then when you instantiate them in 'MyShapes' you would create them like:
Shape t = new Triangle();
That way you're saying "t is an instance of an object that implements the Shape interface, and I don't really need to care what t really is, because I know it fulfills the contract of a Shape"
