2
\$\begingroup\$

My code asks the user for the type of conversion, the degrees and spits the result back. It will prompt the user to ask for the type of conversion again if it's invalid. It will also prompt the user to see if he wants to quit or rerun the program again.

I was wondering if there is a more efficient way of doing this. I feel like I have a lot of redundant code on here.

public class Problem4 {

public static double celsiusToFahrenheit(double degree_C)
{
    double fahrenheitResult =  (9*(degree_C) / 5) + 32;
    return fahrenheitResult;
}

public static double fahrenheitToCelsius(double degree_F)
{
    double celsiusResult = 5*(degree_F - 32)/9;
    return celsiusResult;
}

public static void init(){
    Scanner scan = new Scanner(System.in);
    double result;
    char cont= 'y';
    char type;


    while (cont!='q' && cont!='Q')
    {
     System.out.println("Please enter the degree:");
     double degree = scan.nextDouble();

     System.out.println("Now enter the type of conversion (For Celsius, type 'C'. For Fahrenheit, type 'F')");
     type = scan.next().charAt(0);

     if(type!='c' && type!='C' && type!='f' && type!='F')
        {
             System.out.println("Error! You've entered an invalid conversion type!");
             System.out.println("Please enter a valid selection (Type C for celsius and F for fahrenheit");
             type = scan.next().charAt(0);
        }
     if (type == 'F' || type =='f')
        {
             result = fahrenheitToCelsius(degree);
             System.out.println("The conversion of "+ degree+"°F "+ "to Celsius is: " + result + "°C");

        }
     if (type == 'C' || type =='c')
        {
             result = celsiusToFahrenheit(degree);
             System.out.println("The conversion of "+ degree+"°C "+ "to Fahrenheit is: " + result + "°F");
        }


        System.out.println("Would you like to quit? Type 'q' to quit or type any other letter to continue:");
        cont = scan.next().charAt(0);
    }
    scan.close();
}

public static void main(String[] args) 
{
    init();
}
}
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

I'm not excited about the repetition in this sequence:

 if(type!='c' && type!='C' && type!='f' && type!='F')
    {
         System.out.println("Error! You've entered an invalid conversion type!");
         System.out.println("Please enter a valid selection (Type C for celsius and F for fahrenheit");
         type = scan.next().charAt(0);
    }
 if (type == 'F' || type =='f')
    {
         result = fahrenheitToCelsius(degree);
         System.out.println("The conversion of "+ degree+"°F "+ "to Celsius is: " + result + "°C");

    }
 if (type == 'C' || type =='c')
    {
         result = celsiusToFahrenheit(degree);
         System.out.println("The conversion of "+ degree+"°C "+ "to Fahrenheit is: " + result + "°F");
    }

I think I'd prefer:

if (type == 'c' || type == 'C')
     // ...
else if (type == 'f' || type == 'F')
     // ...
else 
    // Error

Although it conflicts somewhat with the preceding, I'm also less than excited about your error handling. You check for a valid input ([cCfF]), and get the next character of input if the first is invalid, but then you don't check whether that's valid or not. I'd rather do something like:

system.out.println("Please enter the conversion (C or F)");
do { 
    type = scan.next().charAt(0);
} while (type != 'c' && type != 'C' && type != 'f' && type != 'F');

I'd also consider using a case statement instead of if statements:

switch (type) { 
    case 'F':
    case 'f':
        result = fahrenheitToCelsius(degree);
        break;
    case 'c':
    case 'C':
         result = celsiusToFahrenheit(degree);
         break;
    default:
         // handle error
}

I'm also a little less than excited about the wording you used. It's not immediately apparent (at least to me) that: For Celsius, type 'C'. For Fahrenheit, type 'F' means that "C" of "F" designates the type of the input rather than the type of the intended result.

If at all possible, I'd prefer to support command line input. Something like "convert 10c" seems a lot easier to use (and script, etc.) than having to read and respond to a couple of prompts to tell it what I want.

\$\endgroup\$
3
\$\begingroup\$

I'll start by the main methods:

public static double celsiusToFahrenheit(double degree_C)
{
    double fahrenheitResult =  (9*(degree_C) / 5) + 32;
    return fahrenheitResult;
}

public static double fahrenheitToCelsius(double degree_F)
{
    double celsiusResult = 5*(degree_F - 32)/9;
    return celsiusResult;
}
  • You should strive to respect the Java naming conventions: in this case, the variable names should be degreeC and degreeF.
  • In Java, the brackets generally follow the K&R style where the brackets start at the end of the method (and not on a new line).
  • There is no need to store the result in a temporary variable: you can just return it. That makes the code both shorter and clearer.
  • Consider adding spaces before each operator: it makes the code easier to read.
  • You can remove the extra parentheses where they are not needed.

The methods would become:

public static double celsiusToFahrenheit(double degreeC) {
    return 9 * degreeC / 5 + 32;
}

public static double fahrenheitToCelsius(double degreeF) {
    return 5 * (degreeF - 32) / 9;
}

I'll now focus on the code asking the user input.

  • When the users is inputing 2 incorrect values, you are not prompting them again for a new choice. Instead, you directly ask if they want to quit or not. This is because you're not looping until the user enters a correct value. This leads to a weird scenario: if the user wants to quit, they can't and they have to enter wrong values to do so! If would be better to prompt the user for, either a C or a F for something to convert, or a Q if they want to quit.
  • You are checking each character with its uppercase and lowercase variant. You could simplify this by always uppercasing the character the user inputs with Character.toUpperCase(ch) and only check the uppercase variants.
  • This may be because of the way you formatted the code in your question but you should take care about the indentation of your code.
\$\endgroup\$

Your Answer

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

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