Welcome to the Java Programming Forums


The professional, friendly Java community. 21,500 members and growing!


The Java Programming Forums are a community of Java programmers from all around the World. Our members have a wide range of skills and they all have one thing in common: A passion to learn and code Java. We invite beginner Java programmers right through to Java professionals to post here and share your knowledge. Become a part of the community, help others, expand your knowledge of Java and enjoy talking with like minded people. Registration is quick and best of all free. We look forward to meeting you.


>> REGISTER NOW TO START POSTING


Members have full access to the forums. Advertisements are removed for registered users.

Results 1 to 9 of 9

Thread: Please help me clean up my code!

  1. #1
    Member Java Neil's Avatar
    Join Date
    Jan 2011
    Posts
    72
    Thanks
    3
    Thanked 0 Times in 0 Posts

    Default 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


  2. #2
    Banned
    Join Date
    May 2010
    Location
    North Central Illinois
    Posts
    1,631
    My Mood
    Sleepy
    Thanks
    390
    Thanked 112 Times in 110 Posts

    Default Re: Please help me clean up my code!

    It would seem that you're saying that it's supposed to return true if side1 + side2 = side3 and also return false

    if
    side1 + side2 = side3;

    I think it'll return true every time as that's what comes first.

    Also would get rid of second if statement in that boolean method.

    If all three aren't valid, your if should get it to return false.

    If it makes it past your if statement without returning anything, obviously it should return true.

    So change that second if to
    else
    return true;
    Last edited by javapenguin; February 24th, 2011 at 06:48 PM.

  3. #3
    Member Java Neil's Avatar
    Join Date
    Jan 2011
    Posts
    72
    Thanks
    3
    Thanked 0 Times in 0 Posts

    Default Re: Please help me clean up my code!

    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;
          }
     
       }

  4. #4
    Banned
    Join Date
    May 2010
    Location
    North Central Illinois
    Posts
    1,631
    My Mood
    Sleepy
    Thanks
    390
    Thanked 112 Times in 110 Posts

    Default Re: Please help me clean up my code!

    Looks good to me.

  5. #5
    Forum VIP
    Join Date
    Jul 2010
    Posts
    1,707
    Thanks
    25
    Thanked 338 Times in 313 Posts

    Default Re: Please help me clean up my code!

    You actually don't even need the else statement. Think about it logically for a second. If the if statement is true, it will enter the if statement and will ultimately return true (thus leaving the method). If, however, the statement returns false, it will continue to either an else or the reset of the method. So just saying:
    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;
     
          }
    also works. I dunno if it provides any performance advantages, but the else statement is unnecessary nonetheless.
    Last edited by aussiemcgr; February 24th, 2011 at 09:07 PM.
    NOTE TO NEW PEOPLE LOOKING FOR HELP ON FORUM:

    When asking for help, please follow these guidelines to receive better and more prompt help:
    1. Put your code in Java Tags. To do this, put [highlight=java] before your code and [/highlight] after your code.
    2. Give full details of errors and provide us with as much information about the situation as possible.
    3. Give us an example of what the output should look like when done correctly.

    Join the Airline Management Simulation Game to manage your own airline against other users in a virtual recreation of the United States Airline Industry. For more details, visit: http://airlinegame.orgfree.com/

  6. #6
    Banned
    Join Date
    May 2010
    Location
    North Central Illinois
    Posts
    1,631
    My Mood
    Sleepy
    Thanks
    390
    Thanked 112 Times in 110 Posts

    Default Re: Please help me clean up my code!

    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.

  7. #7
    Member Java Neil's Avatar
    Join Date
    Jan 2011
    Posts
    72
    Thanks
    3
    Thanked 0 Times in 0 Posts

    Default Re: Please help me clean up my code!

    Quote Originally Posted by javapenguin View Post
    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!

  8. #8
    Forum VIP
    Join Date
    Jul 2010
    Posts
    1,707
    Thanks
    25
    Thanked 338 Times in 313 Posts

    Default Re: Please help me clean up my code!

    Quote Originally Posted by javapenguin View Post
    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.
    Oh, by bad. The if statement will return false if the condition is met, not true. Regardless, the else statement still isn't needed.
    NOTE TO NEW PEOPLE LOOKING FOR HELP ON FORUM:

    When asking for help, please follow these guidelines to receive better and more prompt help:
    1. Put your code in Java Tags. To do this, put [highlight=java] before your code and [/highlight] after your code.
    2. Give full details of errors and provide us with as much information about the situation as possible.
    3. Give us an example of what the output should look like when done correctly.

    Join the Airline Management Simulation Game to manage your own airline against other users in a virtual recreation of the United States Airline Industry. For more details, visit: http://airlinegame.orgfree.com/

  9. The Following User Says Thank You to aussiemcgr For This Useful Post:

    javapenguin (February 25th, 2011)

  10. #9
    Banned
    Join Date
    May 2010
    Location
    North Central Illinois
    Posts
    1,631
    My Mood
    Sleepy
    Thanks
    390
    Thanked 112 Times in 110 Posts

    Red face Re: Please help me clean up my code!



    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;
     
     
          }