patternjavaMinor
Pool volume estimator
Viewed 0 times
poolestimatorvolume
Problem
I am looking to make the following code more efficient since it it rather lengthy. However I need to include in the code constructor classes that include accessor methods for the container number, height, radius 1, radius 2, and a method that computes the liquid that the container can hold. So I am looking to leave these concepts in there but make it more efficient if it is possible.
```
public class PoolCleanRequest {
final static double PI = 3.14;
private String num;
private double height;
private String height1;
private String height2;
private String radius1;
private String radius2;
private double r1;
private double r2;
private String rad2;
private String rad3;
public PoolCleanRequest(String num1){
num = num1;
}
public void setHeight(double h){
height=h;
height1=Double.toString(h);
height2=Double.toString(h);
}
public void setR1(double rr){
r1 = rr;
radius1=Double.toString(rr);
radius2=Double.toString(rr);
}
public void setR2(double rrr){
r2 = rrr;
rad2=Double.toString(rrr);
rad3=Double.toString(rrr);
}
public String getNum(){
return num;
}
public String getHeight1(){
return height1;
}
public String getHeight2(){
return height2;
}
public String getRadius1(){
return radius1;
}
public String getRadius2(){
return radius2;
}
public String getRad1(){
return rad2;
}
public String getRad3(){
return rad3;
}
public double getPoolCost(){
return ((PIheight/3)(r1r1+r2r2+r1*r2))/1000;
}
public static void main( String[] args ) {
PoolCleanRequest pool1 = new PoolCleanRequest("1.0");
PoolCleanRequest pool2 = new PoolCleanRequest("2.0");
PoolCleanRequest height1 = new PoolCleanRequest("20.0");
PoolCleanRequest height2= new PoolCleanRequest("15.0");
PoolCleanRequest radius1 = new PoolCleanRequest("3.0");
PoolCleanRequest radius2 = new PoolCleanRequest("5.0");
PoolCleanRequest rad1 = new PoolCleanRequest("2.0");
PoolCleanRequest rad3 = new PoolCleanRequest("4.0");
pool1
```
public class PoolCleanRequest {
final static double PI = 3.14;
private String num;
private double height;
private String height1;
private String height2;
private String radius1;
private String radius2;
private double r1;
private double r2;
private String rad2;
private String rad3;
public PoolCleanRequest(String num1){
num = num1;
}
public void setHeight(double h){
height=h;
height1=Double.toString(h);
height2=Double.toString(h);
}
public void setR1(double rr){
r1 = rr;
radius1=Double.toString(rr);
radius2=Double.toString(rr);
}
public void setR2(double rrr){
r2 = rrr;
rad2=Double.toString(rrr);
rad3=Double.toString(rrr);
}
public String getNum(){
return num;
}
public String getHeight1(){
return height1;
}
public String getHeight2(){
return height2;
}
public String getRadius1(){
return radius1;
}
public String getRadius2(){
return radius2;
}
public String getRad1(){
return rad2;
}
public String getRad3(){
return rad3;
}
public double getPoolCost(){
return ((PIheight/3)(r1r1+r2r2+r1*r2))/1000;
}
public static void main( String[] args ) {
PoolCleanRequest pool1 = new PoolCleanRequest("1.0");
PoolCleanRequest pool2 = new PoolCleanRequest("2.0");
PoolCleanRequest height1 = new PoolCleanRequest("20.0");
PoolCleanRequest height2= new PoolCleanRequest("15.0");
PoolCleanRequest radius1 = new PoolCleanRequest("3.0");
PoolCleanRequest radius2 = new PoolCleanRequest("5.0");
PoolCleanRequest rad1 = new PoolCleanRequest("2.0");
PoolCleanRequest rad3 = new PoolCleanRequest("4.0");
pool1
Solution
The first problem that jumps out at me is that many of your variables are "stringly typed". This class is a calculator, right? Why are you storing your numbers as strings rather than as
Why does the constructor take a
There is no need to redefine π. Just use
Why is there a
I don't think that there is much of a reason to define a bunch of setters and getters. Just define one function that accepts all of the dimensions as parameters, and be done with it.
doubles?Why does the constructor take a
num parameter, and what does it mean?There is no need to redefine π. Just use
Math.PI.Why is there a
getPoolCost() method? Your code would be more flexible if you offered a method to calculate the volume, and a separate method to calculate the cost based on the volume.I don't think that there is much of a reason to define a bunch of setters and getters. Just define one function that accepts all of the dimensions as parameters, and be done with it.
Context
StackExchange Code Review Q#136689, answer score: 5
Revisions (0)
No revisions yet.