patternjavaMinor
Quadratic function solver + testing
Viewed 0 times
solverquadraticfunctiontesting
Problem
I wrote a simple quadratic function solver with unit tests. Can anyone check if this is okay?
Quadratic function class
Result class example
This is a test:
```
public class QuadraticFunctionTest {
@Test
public void testIfEquationHasTwoResults() throws Exception {
QuadraticFunction quadraticFunction = new QuadraticFunction(1,-4,-5);
Result result = new DeltaBiggerThanZero(-1.0,5.0);
assertEquals(quadraticFunction.calculate().getX1(), result.getX1());
assertEquals(quadraticFunction.calculate().getX2(), result.getX2());
}
@Test
public void testIfEquationHasOneResult(){
Quadratic function class
public class QuadraticFunction {
private Integer a,b,c;
public QuadraticFunction(Integer a, Integer b, Integer c) {
this.a = a;
this.b = b;
this.c = c;
}
public Result calculate() throws NullPointerException{
if(calculateDelta()>0){
return new DeltaBiggerThanZero(calculateX1(),calculateX2());
}else if(calculateDelta() == 0){
return new DeltaEqualsZero(calculateOneRoot());
}else{
return new NoSolution();
}
}
private Double calculateDelta(){
return Math.pow(b,2) - 4*a*c;
}
private Double rootOfDelta(){
return Math.sqrt(calculateDelta());
}
private Double calculateX1(){
return (-b-rootOfDelta())/2*a;
}
private Double calculateX2(){
return (-b+rootOfDelta())/2*a;
}
private Double calculateOneRoot(){
return Double.valueOf((-b)/2*a);
}Result class example
final public class DeltaBiggerThanZero implements Result{
private Double x1, x2;
public Double getX1() {
return x1;
}
public Double getX2() {
return x2;
}
public DeltaBiggerThanZero(Double x1, Double x2) {
this.x1 = x1;
this.x2 = x2;
}
public String printSimpleResult() {
return "Rozwiązaniem równiania jest: x1="+x1+" x2="+x2;
}
}This is a test:
```
public class QuadraticFunctionTest {
@Test
public void testIfEquationHasTwoResults() throws Exception {
QuadraticFunction quadraticFunction = new QuadraticFunction(1,-4,-5);
Result result = new DeltaBiggerThanZero(-1.0,5.0);
assertEquals(quadraticFunction.calculate().getX1(), result.getX1());
assertEquals(quadraticFunction.calculate().getX2(), result.getX2());
}
@Test
public void testIfEquationHasOneResult(){
Solution
Looks good, nice and clean. Nitpicks:
You didn't include the code for
On the other hand, creating a class hierarchy for different kinds of solutions is rather problematic for the client (arguably this is not the scenario that calls for inheritance, which should be used judiciously). Since
A
To get the output of:
Rozwiązania to: [-1.0, 5.0]
- no need to introduce a method called
printSimpleResultwhen you can simply overridetoString(either way, the "print" part is a lie as the method returns the string, doesn't print it)
- missing
@Overrideannotation inDeltaBiggerThanZero
- naming a variable "result" in the tests is a bit ambiguous, especially since it seems that you don't know what order of params
assertEqualstakes in (it's "expected, actual", for the record)
- why artificially limit input parameters
a,b,cto integers only?
- what's the purpose of
throws NullPointerExceptionincalculate's signature? I see no way in which this exception could be triggered, and you're not required to declare unchecked exceptions (and it's a matter of discussion whether you should)
You didn't include the code for
NoSolution but apparently it's an implementation that returns null for most of the fields. This begs for using Java 8's Optional. Returning null and hoping the clients will know and remember how to interpret it is a bad idea.On the other hand, creating a class hierarchy for different kinds of solutions is rather problematic for the client (arguably this is not the scenario that calls for inheritance, which should be used judiciously). Since
calculate() returns an abstract Result, the client still needs to do "sniffing" on the returned object to figure out how many (if any) results there were. They could to this either by checking via instanceof or trying to see what x1 and x2 are (if x2 is null it must be the "delta-is-zero" case, if both x1 and x2 are null it must be the "delta-is-negative" case with no solutions). Neither of these options is elegant or clean. A better solution could be to return a List that has either zero, one or two elements in it. A simple implementation that would allow you to get rid of the Result type and its subclasses could be:public List calculate() {
Double delta = calculateDelta();
if (delta > 0) {
return new ArrayList(Arrays.asList(new Double[] { calculateX1(), calculateX2() }));
} else if (delta == 0) {
return new ArrayList(Arrays.asList(new Double[] { calculateOneRoot() }));
} else {
return Collections. emptyList();
}
}A
List has a nice default implementation of toString() so the client calling the code above could just do:QuadraticFunction qf = new QuadraticFunction(1, -4, -5);
List results = qf.calculate();
System.out.println("Rozwiązania to: " + results);To get the output of:
Rozwiązania to: [-1.0, 5.0]
Code Snippets
public List<Double> calculate() {
Double delta = calculateDelta();
if (delta > 0) {
return new ArrayList<Double>(Arrays.asList(new Double[] { calculateX1(), calculateX2() }));
} else if (delta == 0) {
return new ArrayList<Double>(Arrays.asList(new Double[] { calculateOneRoot() }));
} else {
return Collections.<Double> emptyList();
}
}QuadraticFunction qf = new QuadraticFunction(1, -4, -5);
List<Double> results = qf.calculate();
System.out.println("Rozwiązania to: " + results);Context
StackExchange Code Review Q#117792, answer score: 5
Revisions (0)
No revisions yet.