trjhtr

I was given a program that generates XML, filters and sorts the elements. The program was rather terrible on purpose because it is an assignment. The original output looked like the following:

<geometricfigures><figure><type>Square</type><area>0,661769</area><xcoordinate>2</xcoordinate><ycoordinate>0</ycoordinate></figure><figure><type>Circle</type><area>2,792598</area><xcoordinate>10</xcoordinate><ycoordinate>9</ycoordinate></figure><figure><type>Triangle</type><area>34,830437</area><xcoordinate>13</xcoordinate><ycoordinate>3</ycoordinate></figure><figure><type>Square</type><area>59,365646</area><xcoordinate>7</xcoordinate><ycoordinate>17</ycoordinate></figure><figure><type>Rectangle</type><area>87,852760</area><xcoordinate>2</xcoordinate><ycoordinate>5</ycoordinate></figure><figure><type>Triangle</type><area>110,448134</area><xcoordinate>11</xcoordinate><ycoordinate>5</ycoordinate></figure><figure><type>Rectangle</type><area>199,078916</area><xcoordinate>18</xcoordinate><ycoordinate>4</ycoordinate></figure><figure><type>Rectangle</type><area>244,638675</area><xcoordinate>10</xcoordinate><ycoordinate>18</ycoordinate></figure><figure><type>Circle</type><area>261,236815</area><xcoordinate>12</xcoordinate><ycoordinate>0</ycoordinate></figure><figure><type>Circle</type><area>309,741691</area><xcoordinate>17</xcoordinate><ycoordinate>12</ycoordinate></figure></geometricfigures>

I have written a "better" program that does the same(?) output in safe ways. You don't have to see the original code because it isn't mine. The following is my code to solve the assignment in a neat way.

Figure.java

import javax.xml.bind.annotation.XmlAccessType;
import javax.xml.bind.annotation.XmlAccessorType;
import javax.xml.bind.annotation.XmlRootElement;

@XmlRootElement(name = "figure")
@XmlAccessorType(XmlAccessType.FIELD)
public class Figure 
 public int getxCoordinate() 
 return xCoordinate;
 

 public void setxCoordinate(int xCoordinate) 
 this.xCoordinate = xCoordinate;
 

 public int xCoordinate;

 public int getyCoordinate() 
 return yCoordinate;
 

 public void setyCoordinate(int yCoordinate) 
 this.yCoordinate = yCoordinate;
 

 public int yCoordinate;
 public String type;

 public double area;

 public String getType() 
 return type;
 

 public void setType(String type) 
 this.type = type;
 

 public double getArea() 
 return calculateArea();
 
 public double calculateArea() 
 return area;
 

 public void setArea(double area) 
 this.area = area;
 



 public int getId() 

 return xCoordinate;
 

 public void setId(int id) 
 this.xCoordinate = id;
 

Figures.java

import javax.xml.bind.annotation.XmlAccessType;
import javax.xml.bind.annotation.XmlAccessorType;
import javax.xml.bind.annotation.XmlElement;
import javax.xml.bind.annotation.XmlRootElement;
import java.util.List;

@XmlRootElement(name = "geometricfigures")
@XmlAccessorType(XmlAccessType.FIELD)
public class Figures

 @XmlElement(name = "figure")
 private List<Figure> figures = null;

 public List<Figure> getFigures() 
 return figures;
 

 public void setFigures(List<Figure> figures) 
 this.figures = figures;
 

Rectangle, Circle, Triangle, Square.java like the following

public class Rectangle extends Figure implements Shape 

 private double x;

 private double y;

 private int posx;

 private int posy;

 public Rectangle(double x, double y, int posx, int posy) 
 this.x = x;
 this.y = y;
 this.xCoordinate=posx;
 this.yCoordinate=posy;
 this.posx = posx;
 this.posy = posy;
 

 public double calculateArea() 
 return x * y;
 
 public void setArea() 
 this.area= x * y;
 

 public int getXCoordinate() 
 return posx;
 

 public int getYCoordinate() 
 return posy;
 

Main.java

import javax.xml.bind.JAXBContext;
import javax.xml.bind.JAXBException;
import javax.xml.bind.Marshaller;

public class Main 
 static Figures figures = new Figures();

 static 
 figures.setFigures(new ArrayList<Figure>());

 int count = 0;
 // Bygg upp data
 for (int i = 0; i < 10; i++) 
 Random r = new Random();
 double b = r.nextDouble() * 20; // Bredd
 double h = r.nextDouble() * 20; // Höjd
 int posx = r.nextInt(20); // Xposition
 int posy = r.nextInt(20); // YPosition


 if (count == 0) 
 Figure e = new Circle(b / 2, posx, posy);
 e.setType("circle");
 ((Circle) e).setArea();
 figures.getFigures().add(e);
 else if (count == 1) 
 Figure e = new Rectangle(b, h, posx, posy);
 e.setType("rectangle");
 ((Rectangle) e).setArea();
 figures.getFigures().add(e);
 else if (count == 2) 
 Figure e = new Square(b, posx, posy);
 e.setType("square");
 ((Square) e).setArea();
 figures.getFigures().add(e);
 else if (count == 3) 
 Figure e = new Triangle(b, h, posx, posy);
 e.setType("triangle");
 ((Triangle) e).setArea();
 figures.getFigures().add(e);
 

 if (count == 3) 
 count = 0;
 else 
 count++;
 
 

 //System.out.println("Before" + figures.getFigures());
 // Remove figures which are within 1 unit distance
 Iterator<Figure> itr = figures.getFigures().iterator();
 while (itr.hasNext()) 
 Object o = itr.next();
 for (Object o2 : figures.getFigures()) 
 if (!o.equals(o2) && distance(o, o2) <= 1) 
 itr.remove();
 break;
 
 
 

 //System.out.println("After " + figures.getFigures());

 // Sort by area increasing
 Collections.sort(figures.getFigures(), new Comparator<Figure>() 
 public int compare(Figure o1, Figure o2) 
 if (o1.calculateArea() == o2.calculateArea())
 return 0;
 return o1.calculateArea() < o2.calculateArea() ? -1 : 1;
 
 );
 //System.out.println("After " + figures.getFigures());

 

 public static void main(String args) throws JAXBException, PersistanceWriteException 
 JAXBContext jaxbContext = JAXBContext.newInstance(Figures.class);
 Marshaller jaxbMarshaller = jaxbContext.createMarshaller();

 jaxbMarshaller.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, true);
 ByteArrayOutputStream os = new ByteArrayOutputStream();
 //jaxbMarshaller.marshal(figures, System.out);
 jaxbMarshaller.marshal(figures, os);
 try 
 String aString = new String(os.toByteArray(), "UTF-8");
 System.out.println(aString);
 catch (Exception e) 
 throw new PersistanceWriteException("Error");
 
 //Marshal the figures list in file
 // jaxbMarshaller.marshal(figures, new File("c:/temp/figures.xml"));
 



 public static double distance(Object o, Object o2) 
 double x1 = 0.0d;
 double x2 = 0.0d;
 double y1 = 0.0d;
 double y2 = 0.0d;

 if (o instanceof Circle) 
 Circle c = (Circle) o;
 x1 = c.getXCoordinate();
 y1 = c.getYCoordinate();
 else if (o instanceof Rectangle) 
 Rectangle r = (Rectangle) o;
 x1 = r.getXCoordinate();
 y1 = r.getYCoordinate();
 else if (o instanceof Square) 
 Square s = (Square) o;
 x1 = s.getXCoordinate();
 y1 = s.getYCoordinate();
 else if (o instanceof Triangle) 
 Triangle t = (Triangle) o;
 x1 = t.getXCoordinate();
 y1 = t.getYCoordinate();
 

 if (o2 instanceof Circle) 
 Circle c = (Circle) o2;
 x2 = c.getXCoordinate();
 y2 = c.getYCoordinate();
 else if (o2 instanceof Rectangle) 
 Rectangle r = (Rectangle) o2;
 x2 = r.getXCoordinate();
 y2 = r.getYCoordinate();
 else if (o2 instanceof Square) 
 Square s = (Square) o2;
 x2 = s.getXCoordinate();
 y2 = s.getYCoordinate();
 else if (o2 instanceof Triangle) 
 Triangle t = (Triangle) o2;
 x2 = t.getXCoordinate();
 y2 = t.getYCoordinate();
 

 double d = distance(x1, y1, x2, y2);
 return d;
 

 public static double distance(double x1, double y1, double x2, double y2) 
 double x = Math.pow(x1 - x2, 2);
 double y = Math.pow(y1 - y2, 2);
 return Math.sqrt(x + y);
 

Output example

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<geometricfigures>
 <figure>
 <xCoordinate>10</xCoordinate>
 <yCoordinate>3</yCoordinate>
 <type>circle</type>
 <area>13.870408538037795</area>
 </figure>
 <figure>
 <xCoordinate>2</xCoordinate>
 <yCoordinate>4</yCoordinate>
 <type>square</type>
 <area>18.4098066238029</area>
 </figure>
 <figure>
 <xCoordinate>1</xCoordinate>
 <yCoordinate>17</yCoordinate>
 <type>rectangle</type>
 <area>36.95919767574589</area>
 </figure>
 <figure>
 <xCoordinate>18</xCoordinate>
 <yCoordinate>1</yCoordinate>
 <type>square</type>
 <area>42.541412668413486</area>
 </figure>
 <figure>
 <xCoordinate>12</xCoordinate>
 <yCoordinate>11</yCoordinate>
 <type>triangle</type>
 <area>55.28650002737187</area>
 </figure>
 <figure>
 <xCoordinate>0</xCoordinate>
 <yCoordinate>1</yCoordinate>
 <type>circle</type>
 <area>57.192991007832184</area>
 </figure>
 <figure>
 <xCoordinate>15</xCoordinate>
 <yCoordinate>0</yCoordinate>
 <type>rectangle</type>
 <area>109.2909113545273</area>
 </figure>
 <figure>
 <xCoordinate>8</xCoordinate>
 <yCoordinate>18</yCoordinate>
 <type>triangle</type>
 <area>112.23126251037294</area>
 </figure>
 <figure>
 <xCoordinate>13</xCoordinate>
 <yCoordinate>4</yCoordinate>
 <type>circle</type>
 <area>135.37137919412515</area>
 </figure>
 <figure>
 <xCoordinate>15</xCoordinate>
 <yCoordinate>14</yCoordinate>
 <type>rectangle</type>
 <area>237.44416405649417</area>
 </figure>
</geometricfigures>

About your choice of using Jaxb, Jaxb is a powerfull framework to serialize and deserialize Xml to Objects. However, you need to have the whole graph of objects in memory to generate the Xml and that may affect your performances.

For your Rectangle class. There is those two methods, calculateArea():double and setArea():void they do more or less the same but the second one in really strange. First, because usually, a setter expect a parameter. Second, because you have to call it on all kind of shape to have it completely initialized. this is usually a job for the constructor. And you expose the area via a getArea():double that either compute it on each call or return the value precomputed by your constructor (and each time one value change)

The distance method in your main cast all objects while it is unnecessary:
((Circle) o).getXCoordinate(); will return the same result as o.getXCoordinate();.

You can also avoid casting in your static block if you declare the correct type for your variable. Circle c = new Circle().

If you want to remove the shapes with the same area, you can use a Set with a comparator on the area. It will only keep one. The same apply for your sorting.

Finally, in your main method you catch all Exception and rethrow a custom one without reusing the original. By doing that you loose the whole stack trace. It is not a problem in your case but can be when you have a deep structure.

Here are my comments:

Figure class

1. If you have getter and setter methods, then the instance variables should be declared private.




2. grouping the declarations of the variables (either at the top or bottom of the class) helps with readability (the reader doesn't have to search for the variable among all the methods)

Rectangle class

1. I do not understand why you have posx and posy variables. they seem to serve the same purpose of x/yCoordinate variables in the super class.




2. You do realize that Rectangles getXCoordinate() does not override Figures getxCoordinate()?