5
\$\begingroup\$

I just started learning Java and am having difficulty with understanding classes. One of my most recent assignments was to create a class called Triangle that would - given user input from the main method - take in the input to test if the hypothetical "triangle" is equilateral, isosceles, or scalene and then return that to main. I was able to get the program to work.

However, I did not follow the assignment's implementation guidelines, which specified having two additional methods that calculated largest and smallest side-length. I am wondering, after the fact, if anyone can look at what I did and let me know if they can see and or explain - in beginner terms - why having those two methods would be needed if it works without them (i.e. is my program somehow at a disadvantage by nature of NOT having them) and if so, what was I doing wrong that kept throwing errors when I tried to implement those methods?

I'll include both the assignment guidelines (for reference since I do not assume anyone is a mind reader) and then my code:

The assignment guidelines:

Your assignment is to write a class definition (not a program, there is no main method) named Triangle (saved in a file Triangle.java). Triangle has 3 instance variables: int side1, side2, side3; The class Triangle must include the following constructors and methods: (If your class does not contain any of the following methods, points will be deducted).

public Triangle (int s1, int s2, int s3) 

- Sets up a triangle with the specified side lengths.

private int largest() 

- Returns the length of the longest side of the triangle. This is a helper method.

private int shortest() 

- Returns the length of the shortest side of the triangle. This is a helper method.

public boolean is_equilateral() 

- Determines whether a triangle is equilateral. If the longest side is equal to the shortest side, then the triangle is equilateral.

public boolean is_isosceles() 

- Determines whether a triangle is isosceles. Any (and at least) two sides must be equal.

public boolean is_scalene() 

- Determines whether a triangle is scalene. No two sides are equal.

public String toString() 

- Prints the sides of the triangle.

My code:

public class Triangle {

private int side1, side2, side3;   // 3 sides for user-input of triangle side length    

public Triangle (int s1, int s2, int s3){  // instance variables

        side1 = s1;

        side2 = s2;

        side3 = s3;


}

// My largest and shortest helper methods never got off the ground
// I left them in for kicks and giggles as comments

//      private int largest(int s1, int s2, int s3) {   <------ Why are these supposed to be declared as private?
//
//          int max = Math.max(Math.max(s1, s2), s3);                                                                   // and is_scalene to determine what type of     
//          return max;
//          }

//      private int shortest(int s1, int s2, int s3) { 
//        // all the other is_x methods below   
//          int min = Math.min(Math.min(s1, s2), s3);   
//          return min;
//          }


    public boolean is_equilateral() { // These methods will return a true or false back to main()

        int max = Math.max(Math.max(side1, side2),side3);  // Because I couldn't get largest/smallest
        int min = Math.min(Math.min(side1, side2), side3); // to work, I opted to use Java's Math.max/min
                                                           // methods instead.  
        boolean answer = false;     // default answer set to false                              

        if (max == min)     // if the largest # is also equal to the smallest # (i.e. all equal) 
                            // then true else false
        {
            answer = true;
        }

        else
        {
            answer = false;
        }

        return answer;
    }


    public boolean is_isosceles() {

        boolean answer = false; 

        if(side1 == side2 || side1 == side3 || side2 == side3)
        {
            answer = true;  // if any 2 sides are equal then true, else false
        }

        else
        {
            answer = false;
        }

        return answer;

    }

    public boolean is_scalene() {

        boolean answer = false;

        if(side1 != side2 && side2 != side3) 
        {
            answer = true;  // if all #'s are different, then true, else false
        }

        else 
        {
            answer = false;
        }

        return answer;
    }

    public String toString() {  // print results on same line

        return "" + this.side1 + " " + this.side2 + " " + this.side3 + "\n\n";

    }



}

// Then the test file that we use to ensure that the class is working is below

import java.util.Scanner;
public class Assignment4 
{

//===========================================================
// Create and determine properties of various triangles.
//===========================================================
public static void main (String[] args) 
{
  Scanner console = new Scanner(System.in);
  int num1, num2, num3;
  String another;

  // repeat until the user enter 'n'
  do
  {
     // get the input
     System.out.println("Enter the sides of the triangle: ");
     num1 = console.nextInt();
     num2 = console.nextInt();
     num3 = console.nextInt();


     // create the Triangle
     Triangle myTriangle = new Triangle (num1, num2, num3);

     // print the Triangle
     System.out.println(myTriangle.toString() + " triangle:");

     //check the isosceles
     if (myTriangle.is_isosceles())
       System.out.println("It is isosceles");
     else
       System.out.println("It is not isosceles");

     //check the equilateral
     if (myTriangle.is_equilateral())
       System.out.println("It is equilateral");
     else
       System.out.println("It is not a equilateral");

     //check the scalene
     if (myTriangle.is_scalene())
       System.out.println("It is scalene");
     else
       System.out.println("It is not scalene");

     // find if the user want to repeat
     System.out.println();
     System.out.print("Check another Triangle (y/n)? ");
     another = console.next();

  } while (another.equalsIgnoreCase("y"));


  }  // method main

}  // class Assignment4
\$\endgroup\$
  • 5
    \$\begingroup\$ I am annoyed that the required / suggested method names are is_equilateral() instead of the conventional isEquilateral(), and largest() instead of longest(). \$\endgroup\$ – 200_success Sep 14 '17 at 13:56
8
\$\begingroup\$

Why are these supposed to be declared as private?

Private methods can be used only from within the class. Supposedly, whoever wrote the guidelines thought that you'll need to call these methods from the public ones and, for some reason (e.g. to illustrate the difference in access modifiers) didn't want them to be public.

Commented out declarations differ from the guidelines:

//guidelines
private int largest() 
//comment
private int largest(int s1, int s2, int s3)

You don't really need to pass sides to largest. You can use side1, side2 and side3 like you do in public methods.

Opportunity to use helper methods:

Let's look at this line in is_equilateral:

Math.max(Math.max(side1, side2),side3);

Notice that it looks terribly similar to the largest method:

int max = Math.max(Math.max(s1, s2), s3);

Actually, if you uncomment private methods you can use them here:

int max = largest();
int min = shortest();

Handle boolean values directly

I've noticed a pattern of handling boolean values step-by-step. It repeats itself in all three methods:

boolean answer = false
if(*SomeBooleanExpression*) 
{
    answer = true;  // if all #'s are different, then true, else false
}
else 
{
    answer = false;
}
return answer;

You can make this code shorter and more readable, by using boolean values directly. For example, the code above is equivalent to this:

boolean answer = *SomeBooleanExpression*;
return answer;

after that, the temporary variable declaration seems unnecessary:

return *SomeBooleanExpression*;

For example, you can change is_scalene to:

public boolean is_scalene() { 
    return side1 != side2 && side2 != side3;
}

Now, it's easier to spot the bug: is_scalene returns true for 3,5,3.

Further improvements

  • Be consistent with the style. Curly braces placement is particularly inconsistent, at various points you're using each of the three possible options.
  • Replace inline comments with Javadoc comments for methods.
  • Learn unit testing. This goes way beyond your assignment, but it will make it much easier for you to test the code.
\$\endgroup\$
  • 1
    \$\begingroup\$ Good overall comments, to which my only addition would be to replace the main method with proper unit tests. \$\endgroup\$ – Martin Spamer Sep 14 '17 at 8:47
  • 1
    \$\begingroup\$ @MartinSpamer Thanks! I've mentioned unit tests in the last part of the answer. I assumed that OP will need to familiarize themselves with the concept first. \$\endgroup\$ – default locale Sep 14 '17 at 8:54
  • 2
    \$\begingroup\$ One other thing to note: I don't think appending new line characters (\n) in your toString() method is correct. That is the sort of thing that should be performed by the caller if/when it is required. \$\endgroup\$ – Phylogenesis Sep 14 '17 at 10:39
  • 1
    \$\begingroup\$ In case it isn't obvious, based on the advice in this answer, if you followed the implementation suggestion in the assignment, then you could write something as simple as public boolean is_equilateral() { return shortest() == largest(); }. \$\endgroup\$ – 200_success Sep 14 '17 at 13:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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