Thread: Please help me clean up my code!

Hi everyone. I need a little help with an assignment and hopefully learn something in the process. The assignment is to use methods to calculate the area of a triangle. I am to use a boolean method, and a double method. I'm also supposed to do this in JOptionPane.

Here is my code...

   import javax.swing.JOptionPane;
   public class MyTriangle {
      public static void main(String[] args) {
         //Declare everything
         boolean valid = false;
         double side1, side2, side3;
 
      	//Give the user directions
         JOptionPane.showMessageDialog(null, 
            "Directions for Neil's Triangle Area Calculator:\n" +
            "1. Enter three lengths of a triangle\n" +
            "2. The sum of the first two sides of the triangle\n" + "     HAS to be GREATER than the third side\n" +
            "3. HAVE FUN!\n" + "");
 
      	//Prompt user input	
         side1 = Double.parseDouble(JOptionPane.showInputDialog(null,"Please enter the 1st side: "));          
         side2 = Double.parseDouble(JOptionPane.showInputDialog(null,"Please enter the 2nd side: "));            
         side3 = Double.parseDouble(JOptionPane.showInputDialog(null,"Please enter the 3rd side: "));
         if (isValid(side1, side2, side3))
            JOptionPane.showMessageDialog(null, "The area of a Trangle with the sides lengths of \n"
                + side1 + ", " + side2 + ", and " + side3 + " is \n" + area(side1, side2, side3));
         {
            {              
            }       
         }
      }  
   	//Check if the sum of two sides is greater than the third side
      public static boolean isValid(double side1, double side2, double side3) {
         if(((side1 + side2) <= side3) || ((side2 + side3) <= side1) || ((side3 + side1) <= side2)){
            JOptionPane.showMessageDialog(null,"Error: The sum of the first two sides\n " +
               "was NOT GREATER than the third side");      
            return false;   
         }
         if(((side1 + side2) >= side3) || ((side2 + side3) <= side1) || ((side3 + side1) <= side2)){
         }
         return true;
      }
   	//Returns the area of the triangle
      public static double area(double side1, double side2, double side3) {
 
         double s = (side1 + side2 + side3)/2;
         double area = Math.sqrt(s * (s - side1) * (s - side2) * (s - side3));
         return area;
      }
 
   }

The big thing here is not if it runs--it does, but to achieve the results in the most efficient way. My teacher is a stickler for unneeded code, and rightfully so. Anyway, could you guys tear me a new one with what I have written and give me some tips on how to do this the proper way.

I should state that we haven't gotten to arrays if that has any bearing on your advice. That will be the next chapter!

Thanks for you time and knowledge.

Neil

Is this better? I might have misunderstood you...

   import javax.swing.JOptionPane;
   public class MyTriangle {
      public static void main(String[] args) {
         //Declare everything
         boolean valid = false;
         double side1, side2, side3;
 
      	//Give the user directions
         JOptionPane.showMessageDialog(null, 
            "Directions for Neil's Triangle Area Calculator:\n" +
            "1. Enter three lengths of a triangle\n" +
            "2. The sum of the first two sides of the triangle\n" + "     HAS to be GREATER than the third side\n" +
            "3. HAVE FUN!\n" + "");
 
      	//Prompt user input	
         side1 = Double.parseDouble(JOptionPane.showInputDialog(null,"Please enter the 1st side: "));          
         side2 = Double.parseDouble(JOptionPane.showInputDialog(null,"Please enter the 2nd side: "));            
         side3 = Double.parseDouble(JOptionPane.showInputDialog(null,"Please enter the 3rd side: "));
         if (isValid(side1, side2, side3))
            JOptionPane.showMessageDialog(null, "The area of a Trangle with the sides lengths of \n"
                + side1 + ", " + side2 + ", and " + side3 + " is \n" + area(side1, side2, side3));
         {
            {              
            }       
         }
      }  
   	//Check if the sum of two sides is greater than the third side
      public static boolean isValid(double side1, double side2, double side3) {
         if(((side1 + side2) <= side3) || ((side2 + side3) <= side1) || ((side3 + side1) <= side2)){
            JOptionPane.showMessageDialog(null,"Error: The sum of the first two sides\n " +
               "was NOT GREATER than the third side");      
            return false;   
         }  
 
         else{
            return true;
         }
 
      }
   	//Returns the area of the triangle
      public static double area(double side1, double side2, double side3) {
 
         double s = (side1 + side2 + side3)/2;
         double area = Math.sqrt(s * (s - side1) * (s - side2) * (s - side3));
         return area;
      }
 
   }

Looks good to me.

Well...actually the if statement will return false if condition is met.

I don't see how it'll just return true on its own.

Originally Posted by javapenguin

Looks good to me.

Hey, thanks for your help. I have a very good, but hard teacher and I'm having to learn on a learning curve!

I see what you're getting at.

I knew that but somehow it didn't cross my mind.



You can just

 public static boolean isValid(double side1, double side2, double side3) {
         if(((side1 + side2) <= side3) || ((side2 + side3) <= side1) || ((side3 + side1) <= side2)){
            JOptionPane.showMessageDialog(null,"Error: The sum of the first two sides\n " +
               "was NOT GREATER than the third side");      
            return false;   
         }  
         return true;
 
 
      }