0

I am trying to add another error for the if (student == 1,2,3).
And if they input a 0 for the amount of credits they are taking it shows an invalid input message.

Any help on what I am doing wrong?

import javax.swing.*;
import java.text.*;

public class TuitionCost {

  public static void main(String[] args) {
    int costHours;
    int student;
    String input;
    double a;
    double b;
    double c;
    DecimalFormat dollar = new DecimalFormat("#,##0.00");

    JOptionPane.showMessageDialog(null, "OCC Tuition Cost Calculation Program", "Tuition Costs at OCC", JOptionPane.INFORMATION_MESSAGE);
    input = JOptionPane.showInputDialog(null, "Are you a:\n1 - College District Residents\n2 - Non-Residents of College District\n3 - Out-of-State and International Students\n\nPlease enter 1, 2 or 3:", "Input", JOptionPane.QUESTION_MESSAGE);
    student = Integer.parseInt(input);

    if (student == 1) {
      input = JOptionPane.showInputDialog(null, "How many credit hours are you taking?", JOptionPane.QUESTION_MESSAGE);
      costHours = Integer.parseInt(input);
      a = costHours * 76.40;
      JOptionPane.showMessageDialog(null, dollar.format(costHours) + " hours at $76.40 per hour yields a tuition of $" + dollar.format(a));
      System.exit(0);
    }

    if (student == 2) {
      input = JOptionPane.showInputDialog(null, "How many credit hours are you taking?", JOptionPane.QUESTION_MESSAGE);
      costHours = Integer.parseInt(input);
      b = costHours * 139.10;
      JOptionPane.showMessageDialog(null, dollar.format(costHours) + " hours at $139.10 per hour yields a tuition of $" + dollar.format(b));
      System.exit(0);
    }

    if (student == 3) {
      input = JOptionPane.showInputDialog(null, "How many credit hours are you taking?", JOptionPane.QUESTION_MESSAGE);
      costHours = Integer.parseInt(input);
      c = costHours * 195.15;
      JOptionPane.showMessageDialog(null, dollar.format(costHours) + " hours at $195.15 per hour yields a tuition of $" + dollar.format(c));
      System.exit(0);
    } else {
      JOptionPane.showMessageDialog(null, "You must enter a 1, 2, or 3.\nPlease Run the program again.", "Invalid Input", JOptionPane.ERROR_MESSAGE);
    }
    System.exit(0);
  }
}
TwoThe
  • 13,879
  • 6
  • 30
  • 54

6 Answers6

4

This is the proper way to write if-else if-else statements. You are missing "else".

if (student == 1)
{

}
else if (student == 2)
{

}
else if (student == 3)
{

}
else
{

}
korhner
  • 723
  • 4
  • 14
  • You really want to recognize the patter used by all student "types" and do work once .. not type it all out multiple times (my keyboard doesn't spell, so I can create errors on the second or third copy of the same stuff). – ErstwhileIII Feb 22 '14 at 21:50
2

It's better to use switch if you have a set of possible values:

switch (student) {
    case 0:
        //do stuff
        break;
    case 1:
        //do stuff
        break;
    ...
    default:
        //it's analog of else
        //do stuff
        break;
}

http://docs.oracle.com/javase/tutorial/java/nutsandbolts/switch.html

nikis
  • 11,166
  • 2
  • 35
  • 45
  • Again .. recognize what is different between the student "types", in this case ONLY the hourly cost is different. So best practice would be to create an array of the hourly costs and determine the index into that array ... then you only have to write the rest of the statements once. – ErstwhileIII Feb 22 '14 at 21:51
2

I would recommend following the DRY principle for this code. Declare the rates as a double[] (although Double is not recommended for money) and then use the array to get the values for each particular type of student. The code for each student is so similar that a simple array will allow the main logic to be wrote once.

double[] rates = {76.40,139.10,195.15};
student = Integer.parseInt(input);

if(student > 0 && student < 4){
    input = JOptionPane.showInputDialog(null, "How many credit hours are you taking?", JOptionPane.QUESTION_MESSAGE);
    costHours = Integer.parseInt(input);
    if(costHours != 0){
        a = costHours * rates[student];
        JOptionPane.showMessageDialog(null, dollar.format(costHours) + " hours at $" + rates[student] + " per hour yields a tuition of $" + dollar.format(a));
    }else{
        JOptionPane.showMessageDialog(null, "Credit hours cannot be zero.", "Invalid Input", JOptionPane.ERROR_MESSAGE);
    }
    System.exit(0);
}else{
    JOptionPane.showMessageDialog(null, "You must enter a 1, 2, or 3.\nPlease Run the program again.", "Invalid Input", JOptionPane.ERROR_MESSAGE);
}

Gist of Full File

Community
  • 1
  • 1
Kevin Bowersox
  • 93,289
  • 19
  • 159
  • 189
1

You might want to simplify your processing and factor your code into several methods. Consider the following organization:

  1. Put the hourly cost depending on student residence into an array
  2. Create a method to show a message
  3. Create a method to ask a question, get an int response AND check that the response is between min and max

Then you can adjust your code to be a bit more simple -- for example.

package com.snippet;

import java.text.DecimalFormat;
import javax.swing.JOptionPane;

public class TuitionCostNew {
private static TuitionCostNew me;

/** cost per hour for:
 *  college district resident, 
 *  in-state but not college district, 
 *  out of state (including international)
 * 
 */
private static double[] hourCost = { 76.40, 139.10, 195.15 };

/**
 * @param args
 */
public static void main(String[] args) {
    // TODO Auto-generated method stub
    me = new TuitionCostNew();
    me.start(hourCost);
}

/**
 * @param hourCost
 */
private void start(double[] hourCost) {
    int studentType;
    int creditHours;
    double tuitionCost;
    DecimalFormat dollar = new DecimalFormat("#,##0.00");

    showInformationMessage("OCC Tuition Cost Calculation Program","Tuition Costs at OCC");

    studentType = inputIntegerDialog(
            "Are you a:\n1 - College District Residents\n2 - Non-Residents of College District\n3 - Out-of-State and International Students\n\nPlease enter 1, 2 or 3:",
            1, 3);

    creditHours = inputIntegerDialog("How many credit hours are you taking?", 1, 25);
    tuitionCost = hourCost[studentType-1] * creditHours;
    showMessage(dollar.format(creditHours) + " hours at "
            + hourCost[studentType-1] + " per hour yields a tuition of $"
            + dollar.format(tuitionCost));
}

/** Show user an informational message pane, including a title for the pane and a message.
 * @param title
 * @param message
 */
private void showInformationMessage(String title, String message) {
    // TODO Auto-generated method stub
    JOptionPane.showMessageDialog(null, title, message,
            JOptionPane.INFORMATION_MESSAGE);
}

/** Shoe user a simple message
 * @param message
 */
private void showMessage(String message) {
    JOptionPane.showMessageDialog(null, message);
}

/** Ask the user to enter an integer value using the message, where the value must be between min and max
 * @param message
 * @param min
 * @param max
 * @return
 */
private int inputIntegerDialog(String message, int min, int max) {
    String input;
    int value = -1;
    boolean validAnswer = false;

    while (!validAnswer) {
        input = JOptionPane.showInputDialog(null, message, "Input",
                JOptionPane.QUESTION_MESSAGE);
        value = Integer.parseInt(input);
        validAnswer = value >= min && value <= max;
        if (!validAnswer) {
            String errMessage = "Invalid entry. Enter number between "
                    + min + " and " + max + ". Try again.";
            showMessage(errMessage);
        }
    }
    return value;
}

}

ErstwhileIII
  • 4,829
  • 2
  • 23
  • 37
  • 1
    You might want to tell the OP why their code doesn't work (IE answer the question). This isn't [code review](http://codereview.stackexchange.com/). What you are doing, posting criticism on others' answers because they did not suggest a completely different alternative, it's kind of annoying. – Radiodef Feb 22 '14 at 22:08
0

You're missing the "else" between all the ifs. So you're getting this flow:

if (1) { ... }
and if (2) { ... }
and if (3) { ... } else { ... }

You want "or" logic everywhere.

Paul Hicks
  • 13,289
  • 5
  • 51
  • 78
  • You want to simplify and avoid all these if / else constructs. Look at the problem and you will see that the student type only controls hourly credit cost and that it must be 1, 2 or 3. So: use an array with student as an index; and a input method that you give a min and max value to to check for error. That way you can reuse the input method for other items (like number of credit hours between 0 and 25, for example) – ErstwhileIII Feb 22 '14 at 21:59
0

That's because else will catch a value that is different from 1, 2 or 3:

else               
     JOptionPane.showMessageDialog(null, "You must enter a 1, 2, or 3.\nPlease Run the program again.", "Invalid Input", JOptionPane.ERROR_MESSAGE);
     System.exit(0);

You need to add a condition to check for 0:

if (student == 0){}

and use else-if syntax:

if(student==0){
}else if(student==1){
}else if(student==2){
}else if(student==0){
}else{
//anything else
}
ltalhouarne
  • 4,586
  • 2
  • 22
  • 31
  • Don't use complex if then else when you can reduce the problem another way. Also, consider looping when asking for a valid input, rather than aborting and having the user start all over. – ErstwhileIII Feb 22 '14 at 22:00