1
\$\begingroup\$

I am in the process of creating a program that allows a user to input known parts of a right triangle and the calculates all of the unknown parts if it is possible.

I am using a class that I wrote in order to do the calculations. I wanted to know if there is a more efficient solution to this problem or if my code is good as is.

public class RightTriangle 
{
    private double sideA; //Leg
    private double sideB; //Leg
    private double sideC; //Hypotenuse 

    private double alpha; 
    private double beta;

    private int numOfSides;
    private int numOfAngles;

    public RightTriangle(double a, double b, double c, double A, double B)
    {
        numOfSides=0;
        numOfAngles=0;

        sideA=a; if(sideA>0){numOfSides++;}
        sideB=b; if(sideB>0){numOfSides++;}
        sideC=c; if(sideC>0){numOfSides++;}

        alpha=A; if(alpha>0){numOfAngles++;}
        beta=B; if(beta>0){numOfAngles++;}

        if(numOfAngles<2)
        {
            findMissingAngles();
        }

        if(numOfSides<3)
        {
            findMissingSides();
        }

        if(!isPossible())
        {
            sideA=sideB=sideC=alpha=beta=0;
        }
    }

    private void findMissingAngles()
    {
        if(alpha==0)
        {
            calcAlpha();
        }

        if(beta==0)
        {
            calcBeta();
        }
    }

    private void findMissingSides()
    {
        if(sideA==0)
        {
            calcSideA();
        }

        if(sideB==0)
        {
            calcSideB();
        }

        if(sideC==0)
        {
            calcSideC();
        }
    }

    private void calcAlpha()
    {
        if(numOfAngles==1)
        {
            alpha=90-beta;
        }

        else
        {
            if(sideA>0 && sideB>0)
            {
                alpha=Math.toDegrees(Math.atan(sideA/sideB));
            }

            else if(sideB>0 && sideC>0)
            {
                alpha=Math.toDegrees(Math.acos(sideB/sideC));
            }

            else if(sideA>0 && sideC>0)
            {
                alpha=Math.toDegrees(Math.asin(sideA/sideC));
            }
        }
    }

    private void calcBeta()
    {
        beta=90-alpha;
    }

    private void calcSideA()
    {
        if(sideC>0)
        {
            sideA=Math.sin(Math.toRadians(alpha)) * sideC;
        }

        else if(sideB>0)
        {
            sideA=Math.tan(Math.toRadians(alpha)) * sideB;
        }
    }

    private void calcSideB()
    {
        sideB=sideA/Math.tan(Math.toRadians(alpha));
    }

    private void calcSideC()
    {
        sideC=sideA/Math.sin(Math.toRadians(alpha));
    }

    private boolean isPossible()
    {
        return ((alpha+beta+90)==180 && Math.abs(Math.sqrt(Math.pow(sideA, 2) + Math.pow(sideB, 2)) - sideC) < 1);
    }

    public void printInfo()
    {
        System.out.println("Triangle Info\n");
        System.out.println("Side A: " + String.format("%.02f", sideA));
        System.out.println("Side B: " + String.format("%.02f", sideB));
        System.out.println("Side C: " + String.format("%.02f", sideC) + "\n");
        System.out.println("Alpha: " + String.format("%.01f", alpha));
        System.out.println("Beta: " + String.format("%.01f", beta));
    }
}

Input

Enter Triangle Information (Enter 0 for missing parts)
Enter side A: 60
Enter side B: 60
Enter side C: 0

Enter angle Alpha: 0 
Enter angle Beta: 0

Output

Triangle Info

Side A: 60.00
Side B: 60.00
Side C: 84.85

Alpha: 45.0
Beta: 45.0
\$\endgroup\$
1
  • \$\begingroup\$ Could you add a usage example of how to use your code? Also an example input/output would be appreciated. It makes your question both more interesting and easier to review. \$\endgroup\$ Jan 29, 2016 at 21:02

2 Answers 2

1
\$\begingroup\$

Keep it simple

        numOfSides=0;
        numOfAngles=0;

        sideA=a; if(sideA>0){numOfSides++;}
        sideB=b; if(sideB>0){numOfSides++;}
        sideC=c; if(sideC>0){numOfSides++;}

        alpha=A; if(alpha>0){numOfAngles++;}
        beta=B; if(beta>0){numOfAngles++;}

        if(numOfAngles<2)
        {
            findMissingAngles();
        }

        if(numOfSides<3)
        {
            findMissingSides();
        }

This seems more complicated than necessary. Why have numOfAngles and numOfSides at all? You could just say

        sideA = a;
        sideB = b;
        sideC = c;

        alpha = A;
        beta = B;

        findMissingAngles();

        findMissingSides();

And then modify

        if(numOfAngles==1)

in calcAlpha to say

        if (beta > 0)

That saves two object fields that exist just for construction purposes.

Be careful of false modularity

    private void calcBeta()
    {
        beta=90-alpha;
    }

This only works if you calculate alpha first. That's the programming logic here, but nothing enforces it. So why have a separate method to do an assignment based on one subtraction? You could change

        if(beta==0)
        {
            calcBeta();
        }

to say

        if (beta==0)
        {
            beta = 90 - alpha;
        }

instead. Then you could get rid of calcBeta.

This also avoids the problem of calling calcBeta without testing that beta doesn't already have a value.

Alternately, you could change calcBeta to be more like calcAlpha where it calculates alpha from beta if beta exists or from two sides otherwise.

Similarly, all the calcSide methods assume that alpha exists. And calcSideB and calcSideC assume that sideA exists. If any of that is not true, then you'll get weird results.

Exceptional circumstances

    private boolean isPossible()
    {
        return ((alpha+beta+90)==180 && Math.abs(Math.sqrt(Math.pow(sideA, 2) + Math.pow(sideB, 2)) - sideC) < 1);
    }

Why not change (alpha+beta+90)==180 to just (alpha + beta) == 90?

Why is 1 the acceptable error? Why not .1 or .03 or something else?

Are negative values acceptable? Because you accept them if they have the right magnitude. For example, -95 and 185 are a valid pair of angles. And -3, -4, 5 are valid side values.

\$\endgroup\$
0
\$\begingroup\$

Constructor

Do not make calculations in a constructor.

Responsibilities

As you only have one class of objects that are not communicating to other objects this program can be considered as a procedural program. OOP is about structuring a program by responsibilities. So let's start to identify them.

The purpose of the program is to calculate missing elements of a right triangle. So you have

  1. Some information with that you may or may not be able to construct a right triangle
  2. An algorithm that identifies if a construction with the given information is possible
  3. A strategy to calculates missing elements
  4. A representation of a valid right triangle

Each of the responsibilities should be represented in a separated code fragment, a method or a class.

1

To calculate missing elements of a right triangle you have to have at least two more information beside GAMMA that is considered to be 90 degrees. Given angles must be in a range of 0 > angle < 90 and edges must be x > 0. You have to decide what to do with redundant and compromising information.I suggest to throw an exception if something is strange.

2

I do not really understand the isPossible()-method. It is called AFTER every calculation and it is calculating something I don't really want to understand. The isPossible() check has to be called as early as possible.

In my reality I first check if something can be done and maybe then it is done. In this case I would first check what I described in (1).

Often if you do things not in the "natural order" you mess up your code.

3

Here you have to identify a strategy to calculate missing information.

You have a lot of if statements to identify the next calculation. That's because you have redundant code that isn't obvious. Look at this:

Math.toDegrees(Math.acos(sideB/sideC));
Math.toDegrees(Math.asin(sideA/sideC));

As ALPHA and BETA are complementary angles this can be written as:

90 - Math.toDegrees(Math.asin(sideB/sideC));
Math.toDegrees(Math.asin(sideA/sideC));

So the Math.asin() is redundant.

Here again some redundant code:

private void calcAlpha()
{
    if(numOfAngles==1)
    {
        alpha=90-beta;
    }
...

private void calcBeta()
{
    beta=90-alpha;
}

Only the names are different but the calculation is the same.

Normally you have 7 cases of given information:

  1. a and b
  2. a and c
  3. b and c
  4. a and alpha
  5. b and alpha
  6. a and beta
  7. b and beta

There is a symmetry you can make use of for this special right triangle case. I suggest to first remember the given names and then "normalize" the given information. After that you are able to reduce the possible cases to 4:

  1. a and b (alpha, beta and c missing)
  2. a and c (alpha, beta and b missing)
  3. a and alpha (beta, b and c missing)
  4. a and beta (alpha, b and c missing)

You only have to remember if the names were switched (normalized).

Normalization is a mechanism so transform information into a state so that an algorithm can rely on certain well defined properties. In this case we eliminate redundancies as it is done within the normalization process of database schemas.

4

At last you have to only have to construct a right triagle with the normalized information.

public class RightTriangle {

    private double a;
    private double b;
    private double alpha;

    public RightTriangle(double a, double b, double alpha) {
        super();
        this.a = a;
        this.b = b;
        this.alpha = alpha;
    }

    public double getAlpha() {
        return alpha;
    }

    public double getBeta() {
        return 90.0 - getAlpha();
    }

    public double getGamma() {
        return 90.0;
    }

    public double getA() {
        return a;
    }

    public double getB() {
        return b;
    }

    public double getC() {
        return Math.sqrt(Math.pow(getA(), 2) + Math.pow(getB(), 2));
    }

}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.