3
\$\begingroup\$

I have currently been working on a program for a few days now, and I am just about done. The goal of my program is to ask the user for the amount they still owe on their credit card, and assign an appropriate monthly interest rate. I have commented the code to make it a bit easier to understand, and I believe it is correct for the most part. My only issue is I do not believed I have called my two other methods properly in Main.

import java.util.*;

/**
* Creates an account statement for a client of a credit card company based
* on their input.
*/

public class CSCard {

private static double interestRate;
private static double newBalance;
private static double interest;
public static void main(String[] args) {
    Scanner keyboard;
    double priorBalance;
    double addtlCharges;
    double newBalance;
    double minPayment;

    // initialize needed variables here
    keyboard = new Scanner(System.in);

    // make sure that the number entered is a double and if so read it in.
    // if not, use a default value of zero. No error message to the user.
    System.out.print("Type the balance: ");
    priorBalance = keyboard.nextDouble();

    if (keyboard.hasNextDouble() == true) {
        interestRate = 0.0;
    } else {
        interestRate = 0.02;
    }


    // make sure that the number entered is a double and if so read it in.
    // if not, use a default value of zero.  No error message to the user.
    System.out.print("\nType the new charge amount: ");
    addtlCharges = keyboard.nextDouble();

    // perform the calculation of interest, new balance and minimum payment
    newBalance = calculateInterest(priorBalance, addtlCharges);
    minPayment = calculateMinPayment(newBalance);

    // output the resulting statement (DO NOT CHANGE)
    System.out.println("\n");
    System.out.println("CS CARD International Statement");
    System.out.println("===============================");
    System.out.printf("Previous balance:   $%,8.2f\n", priorBalance);
    System.out.printf("Additional charges: $%,8.2f\n", addtlCharges);
    System.out.printf("Interest:           $%,8.2f\n\n", interest);
    System.out.printf("New balance:        $%,8.2f\n\n", newBalance);
    System.out.printf("Minimum payment:    $%,8.2f\n", minPayment);
}

/**
 * This method will take in the previous balance and
 * additional charges and compute the amount of interest.
 * If the priorBalance is 0 or less, the interest is 0.
 * If there was a priorBalance, the interest is 2% of the
 * priorBalance plus the additional charges.
 * 
 * @param priorBalance balance before the new charges are added
 * @param addtlCharges charges added this month
 * @return amount of interest to charge
 */
public static double calculateInterest(double priorBalance,
                                       double addtlCharges) {
    // first create a stub and get the input and output working
    // then replace the stub later on with the calculation
    interest = (priorBalance + addtlCharges) * interestRate;
    return interest;
}

/**
 * This method will take in the previous balance and
 * additional charges and compute the minimum payment.
 *          $0.00          for new balance less than $0
 *      new balance        for new balance between $0 and $49.99 (inclusive)
 *         $50.00          for new balance between $50 and $300 (inclusive)
 * 20% of the new balance  for new balance over $300
 *
 * @param balance after interest and charges are added
 * @return minimum payment amount
 */
public static double calculateMinPayment(double balance) {
    // first create a stub and get the input and output working
    // then replace the stub later on with the calculation
    if (newBalance < 0) {
        balance = 0;
    }
    if (newBalance >= 0 && newBalance <= 49.99) {
        balance = newBalance;
    }
    if (newBalance >= 50 && newBalance <= 300) {
        balance = 50.0;
    }
    if (newBalance > 300) {
        balance = (newBalance * 0.2) + newBalance;
    }
    return balance;

}

}
\$\endgroup\$
5
  • \$\begingroup\$ Could you fix your formatting, and only include the code that's related to your issue? (Removing user input and output is a good start.) I'm also not sure what you mean by "calling them properly", and how it's related to your post title. Have you tested your code and is it actually behaving in a way you weren't expecting? \$\endgroup\$
    – millimoose
    Feb 27 '14 at 18:28
  • \$\begingroup\$ Well, if you have a method that calculates interest, and you're assigning the result into a variable named balance, then either you're not calling the methods correctly or you're choosing some really bad names. \$\endgroup\$
    – ajb
    Feb 27 '14 at 18:30
  • \$\begingroup\$ move this 2 method (calculateMinPayment & calculateInterest) in another class, something like Calculate \$\endgroup\$
    – Mostafa Jamareh
    Feb 27 '14 at 18:35
  • \$\begingroup\$ Method calculateMinPayment takes a parameter that it never uses. It is passed newBalance by main but it refers to the static field newBalance directly, and it only uses its parameter balance as a local variable in which it calculates its return value. \$\endgroup\$ Feb 27 '14 at 19:11
  • \$\begingroup\$ @Austin You can always flag the question for mod attention to have it migrated, along with the comments and eventual answers that come here. \$\endgroup\$
    – millimoose
    Feb 27 '14 at 19:33
3
\$\begingroup\$

Watch your scope - in your class you keep your data in two places - local variables, and static members. I cannot see any thought behind choosing where to put them - you pass some around in your method signatures, but then you use the others in the same methods... The most acute case is in calculateMinPayment where you pass the static member newBalance as a parameter and use it inside the method, while totally ignoring (and overriding) the parameter...

In the use-case above, in the name of encapsulation, I believe the correct way is to only keep the state as local variables, passing the needed data to the stateless helper methods, rather than keeping anything in the static state.

Comments - I personally think that comments are a code smell. When the code is clean and readable - comments are superfluous. You've got a lot of comments, and some are more confusing than helpful, since you put them in the wrong location:

// make sure that the number entered is a double and if so read it in.
// if not, use a default value of zero. No error message to the user.
System.out.print("Type the balance: ");
priorBalance = keyboard.nextDouble();

if (keyboard.hasNextDouble() == true) {
    interestRate = 0.0;
} else {
    interestRate = 0.02;
}

The above comments should be placed after setting priorBalance, since they describe interestRate... They are more confusing, since the code does not do what the comment say - it sets the interestRate at either 0.0 or 0.02, then totally ignores the given information, and tries to get nextDouble() even if the last statement returned false.

BTW - don't use if (keyboard.hasNextDouble() == true) - if (keyboard.hasNextDouble()) does exactly the same, and is more readable.

// make sure that the number entered is a double and if so read it in.
// if not, use a default value of zero.  No error message to the user.
System.out.print("\nType the new charge amount: ");
addtlCharges = keyboard.nextDouble();

Was that a result of an unfortunate copy+paste? Again - comment is not helpful, only confuses.

I also believe the prompt Type the new charge amount will appear only after the user entered it, since you check hasNextDouble() before you print the prompt.

// first create a stub and get the input and output working
// then replace the stub later on with the calculation

I don't even begin to understand what this means, and what it has to do with the code it describes... not the second time either...

Naming - give variables and methods names which imply their role in your code - what does this line do?

newBalance = calculateInterest(priorBalance, addtlCharges);

Calculates interest or the new balance?

Also, don't be lazy say - additionalCharges - there are no fines for long names.

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

Your code doesn't quite work yet, due to some confusion when prompting for the interest rate. It will appear to hang at if (keyboard.hasNextDouble() == true) … since System.in has not reached EOF, yet the user has not been prompted to enter the interest rate.

Your main problem is confusion regarding the use of variables. In particular, you have a static variable newBalance that is shadowed in main() by a local variable also named newBalance. For now, I recommend keeping things simple: use local variables only. Also, assign values at the same time that you declare variables whenever possible.

You asked about how to return a value from calculateMinPayment(), and indeed there are some problems there. At first glance, since balance is a parameter, you would expect that the result of the function would vary according to the balance. What actually happens is a lot weirder. You are actually using balance as a local variable; instead, the static variable newBalance is the input.

Another problem with calculateMinPayment() is that the cases don't completely cover all of the cases. What if the balance is $49.995? (The added interest could result in such oddball numbers.) You should probably interpret "between $0 and $49.99 (inclusive)" to mean "between $0 (inclusive) and $50 (exclusive)". Then, chain all the conditions together with if-elseif-elseif-else to make it clear that all cases are covered.

public class CSCard {

    public static void main(String[] args) {
        // make sure that the number entered is a double and if so read it in.
        // if not, use a default value of zero. No error message to the user.
        try (Scanner in = new Scanner(System.in)) {
            double priorBalance = promptDouble(in, "Type the balance: ");
            double interestRate = promptDouble(in, "Interest rate (e.g., 0.02): ");
            double addtlCharges = promptDouble(in, "The new charge amount: ");

            // perform the calculation of interest, new balance and minimum payment
            double interest = calculateInterest(...);
            double newBalance = priorBalance + interest;
            double minPayment = calculateMinPayment(newBalance);

            // output the resulting statement (DO NOT CHANGE)
            System.out.println("\n");
            System.out.println("CS CARD International Statement");
            System.out.println("===============================");
            System.out.printf("Previous balance:   $%,8.2f\n", priorBalance);
            System.out.printf("Additional charges: $%,8.2f\n", addtlCharges);
            System.out.printf("Interest:           $%,8.2f\n\n", interest);
            System.out.printf("New balance:        $%,8.2f\n\n", newBalance);
            System.out.printf("Minimum payment:    $%,8.2f\n", minPayment);
        }
    }

    /**
     * Prompts for a double from <tt>System.in</tt>.
     *
     * @return 0.0 if the input is invalid
     */
    private static double promptDouble(Scanner in, String question) {
        try {
            System.out.print(question);
            System.out.flush();
            return in.nextDouble();
        } catch (InputMismatchException defaultToZero) {
            in.next();          // Consume a token
            System.out.println("Invalid input; interpreting it as 0.0.");
            return 0.0;
        } finally {
            System.out.println();
        }
    }

    public static double calculateInterest(...) {
        /* TODO */
    }

    /**
     * This method will take in the previous balance and
     * additional charges and compute the minimum payment.
     *          $0.00          for new balance less than $0
     *      new balance        for new balance between $0 and $49.99 (inclusive)
     *         $50.00          for new balance between $50 and $300 (inclusive)
     * 20% of the new balance  for new balance over $300
     *
     * @param balance after interest and charges are added
     * @return minimum payment amount
     */
    public static double calculateMinPayment(double balance) {
        if (balance < 0) {
            return 0;
        } else if (balance < 50) {
            return balance;
        } else if (balance <= 300) {
            return 50.0;
        } else {
            return 0.2 * balance;
        }
    }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy