6
\$\begingroup\$

I'm fairly new to Java and I'd like to know if there's any way to improve or refactor the prompt on validating integer values for best practices.

import java.util.Scanner;

public class Triangle {
 public static void main(String[] args) {
  int a, b, c;
  Scanner in = new Scanner(System.in);
  
  // Prompt for a
  System.out.print("a: ");
  if (!in.hasNextInt() || (a = in.nextInt()) <= 0) {
   System.out.println("Input must be a positive integer!");
   return;
  }
  
  // Prompt for b
  System.out.print("b: ");
  if (!in.hasNextInt() || (b = in.nextInt()) <= 0) {
   System.out.println("Input must be a positive integer!");
   return;
  }
  
  // Prompt for c
  System.out.print("c: ");
  if (!in.hasNextInt() || (c = in.nextInt()) <= 0) {
   System.out.println("Input must be a positive integer!");
   return;
  }
  
  if (a + b > c && a + c > b && b + c > a) {
   System.out.println("It's a traingle.");
  } else {
   System.out.println("Invalid lengths for a traingle.");
  }
 }
}
\$\endgroup\$
8
\$\begingroup\$

Code Repetition

In your program, you are using the following code three times:

System.out.print("a: ");
if (!in.hasNextInt() || (a = in.nextInt()) <= 0) {
 System.out.println("Input must be a positive integer!");
 return;
}

You should use a method for this:

public static int getUserInput() {
 Scanner in = new Scanner(System.in);
 int number;
 if (!in.hasNextInt() || (number = in.nextInt()) <= 0) {
  System.out.println("Input must be a positive integer!");
  return -1; //Returns -1 for illegal input
 }
 return number;
}

Input validation

It's good that you control, whether the user made a valid input, but this point can be improved further: Your program stops, when the user makes an invalid input. That's not what an user expects:

public static int getUserInput(String var) {
 Scanner sc = new Scanner(System.in);
 int number;
 while(true) {
  try {
   System.out.print(var + ": ");
   number = sc.nextInt();
   if(number <= 0) {
    throw new InputMismatchException();
   }
   break;  
  } 
  catch(InputMismatchException e) {
   System.out.println("Enter a number > 0.");
   sc.nextLine();
  }
 } 
 return number;
}

By using this code, the user will be able to make inputs until making a valid input. If you don't know about try-catch yet, i suggest reading this explanation.


Logic

I would create a seperate method for the "triangle-validation":

public static boolean isTriangle(int a, int b, int c) {
 if (a + b > c && a + c > b && b + c > a) {
  return true;
 } else {
  return false;
 }
}

Style

Please consider using more than one space for indentation. My suggestion is two or even four spaces. This improves the legibility.


Final code

import java.util.Scanner;
import java.util.InputMismatchException;

public class Triangle {
    public static void main(String[] args) {
        int a, b, c;
        Scanner in = new Scanner(System.in);

        // Prompt for a
        a = getUserInput("a");

        // Prompt for b
        b = getUserInput("b");


        // Prompt for c
        c = getUserInput("c");

        if (isTriangle(a, b, c)) {
            System.out.println("It's a triangle.");
        } else {
            System.out.println("Invalid lengths for a triangle.");
        }
    }

    public static boolean isTriangle(int a, int b, int c) {
        if (a + b > c && a + c > b && b + c > a) {
            return true;
        } else {
            return false;
        }
    }

    public static int getUserInput(String var) {
        Scanner sc = new Scanner(System.in);
        int number;
        while(true) {
            try {
                System.out.print(var + ": ");
                number = sc.nextInt();
                if(number <= 0) {
                    throw new InputMismatchException();
                }
                break;  
            }
            catch(InputMismatchException e) {
                System.out.println("Enter a number > 0.");
                sc.nextLine();
            }
        } 
        return number;
    }
}
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Could you not also replace the isTriangle function into a single line? public static boolean isTriangle(int a, int b, int c) { return a + b > c && a + c > b && b + c > a; } \$\endgroup\$ – Alexander Wolters Jul 10 at 16:28
  • \$\begingroup\$ Yes, of course, but I tried to keep it simple, because the questioner is "fairly new to java". \$\endgroup\$ – Philipp Wilhelm Jul 10 at 16:51
  • 1
    \$\begingroup\$ you could also remove a minor typo: traingle vs triangle (2x) *gg \$\endgroup\$ – Martin Frank Jul 15 at 12:17
  • 1
    \$\begingroup\$ @MartinFrank Done! Thanks for the hint. \$\endgroup\$ – Philipp Wilhelm Jul 15 at 12:19
  • 1
    \$\begingroup\$ thank you for apprechiating it! (i hope i was not too mean to you!) \$\endgroup\$ – Martin Frank Jul 16 at 4:59

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.