Credit Calculator

Question

The code has been test to work by the parameters listed below I just wanted to know if there were any details that I missed or blatantly overlooked.

Develop a Java application that determines whether any of several department-store customers has exceeded the credit limit on a charge account. For each customer the following facts are available:

• account number

• balance at the beginning of the month

• total of all items charged by the customer this month

• total of all credits applied to the customers account this month

• allowed credit limit

The program should input all of these facts as integers, calculate the new balance (new balance = beginning balance + charges - credits) For those customers whose credit limit is exceeded, the program should display the message "Credit Limit Exceeded".

import java.util.Scanner;

public class CreditCalc
{
    public static void main(String[] args)
   {
      Scanner input = new Scanner( System.in );

      int accnum = 0;
      int begBalance = 0;
      int itemTotal = 0;
      int creditsApplied = 0;
      int creditLimit = 0;
      int newBalance = 0;

      System.out.println("Welcome to the Credit Calculator");

      System.out.println("Please Enter Customer Account Number (or -1 to quit): "); 
      accnum = input.nextInt();
        if (accnum != -1)
        {
            System.out.println("Please Enter Customer Beginning Balance: "); 
          begBalance = input.nextInt();
            if (begBalance < 0)
            {
                System.out.println("Please enter a positive amount");
                System.out.println("Please Enter Customer Beginning Balance: ");
                begBalance = input.nextInt();   
            }
        }



      while(accnum != -1)
      {
         System.out.println("Please Enter Total of Items Purchased this Month: "); 
         itemTotal = input.nextInt();
            if (itemTotal < 0)
            {
                System.out.println("Please enter a positive amount");
                System.out.println("Please Enter Total of Items Purchased this Month: ");
                itemTotal = input.nextInt();    
            }


         System.out.println("Please Enter Credits applied to Customer Account: "); 
         creditsApplied = input.nextInt();

         System.out.println("Please Enter Customer Credit Limit: "); 
         creditLimit = input.nextInt();


         newBalance = begBalance + itemTotal - creditsApplied;




            if(newBalance > creditLimit)
            {
                System.out.println("Credit Limit Exceeded");
            }



            System.out.println("Please Enter Customer Account Number (or -1 to quit): "); 
              accnum = input.nextInt();
                if (accnum != -1)
                {
                    System.out.println("Please Enter Customer Beginning Balance: "); 
                  begBalance = input.nextInt();
                    if (begBalance < 0)
                    {
                        System.out.println("Please enter a positive amount");
                        System.out.println("Please Enter Customer Beginning Balance: ");
                        begBalance = input.nextInt();   
                    }
                }


      }

        if(accnum < 0)
        {
            System.out.println("Thank You for using Credit Calculator");
            System.out.println("Goodbye");
        }


   }

}

Answer 1

Loop logic

There's definitely something strange going on with your looping. Why do you have so many checks on accnum equalling zero? Why do you ask for account number in multiple places?

As it is, it's hard to even answer that by eyeballing the code, because it's so bulky. So let's start by pulling some things into methods:

private int getAccountNumber(Scanner input) {
    System.out.println("Please Enter Customer Account Number (or -1 to quit): "); 
    return input.nextInt();
}

We can take the same approach with the other input/output sections, to now get a clearer version of your algorithm:

  System.out.println("Welcome to the Credit Calculator");

  accnum = getAccountNumber(input);
  if (accnum != -1)
  {
      begBalance = getBeginningBalance(input);
  }

  while(accnum != -1)
  {
     itemTotal = getItemTotal(input);
     creditsApplied = getCreditsApplied(input);
     creditLimit = getCreditLimit(input);

     newBalance = begBalance + itemTotal - creditsApplied;

     if(newBalance > creditLimit)
     {
         System.out.println("Credit Limit Exceeded");
     }

     accnum = getAccountNumber(input);
     if (accnum != -1)
     {
         begBalance = getBeginningBalance(input);
     }
  }

   if(accnum < 0)
   {
       System.out.println("Thank You for using Credit Calculator");
       System.out.println("Goodbye");
   }

Okay, now we see the problem more clearly, those first lines before the while loop are checked again at the end of the loop. That's definitely something we can improve on, so let's do it:

  System.out.println("Welcome to the Credit Calculator");

  while(true)
  {
     accnum = getAccountNumber(input);
     if (accnum == -1)
     {
         break;
     }

     begBalance = getBeginningBalance(input);
     itemTotal = getItemTotal(input);
     creditsApplied = getCreditsApplied(input);
     creditLimit = getCreditLimit(input);

     newBalance = begBalance + itemTotal - creditsApplied;

     if(newBalance > creditLimit)
     {
         System.out.println("Credit Limit Exceeded");
     }
  }

  System.out.println("Thank You for using Credit Calculator");
  System.out.println("Goodbye");

Notice how I also removed that last check- if we've broken out of that loop, we already know that accnum < 0 is true.

---

Formatting

It's important to pick good conventions and stick with them. One guideline is the language. For example, in Java it's conventional to put curly braces on the same line, rather than a new one (like I did in that getAccountNumber example). It's also conventional to use camelCased variables without too much abbreviation: accountNumber rather than accnum. Make sure you're consistent with your indentation too, as that's one way the eye quickly gauges control flow.

---

Usability

None of your requirements actually say anything about looping until the user enters some special quit code. Is this really what you want? Have you considered just running through the process once? Or looping forever, forcing the user to quit out by using whatever means the console provides for doing so, rather than providing your own mechanism?

This is somewhat a judgement call, but having to enter a specially defined magic number in a particular place isn't a very user-friendly way to quit a program, so either of the above might be a better user experience. They also mean you now have a simpler program- you want to guard against adding extra code or features that don't actually match any of your requirements.

Similarly, do you actually need this question/answer style? Would receiving the information from command line arguments be acceptable? If so, you could cut your entire program down to a few lines.

---

A note on OO

One last thing, since I noticed a few other answers mentioning taking a more OO approach, I'm going to disagree.

In my opinion one of the best ways to make sure you misunderstand OO is to apply it where it's not appropriate. Here your entire application is concerned with input/output to the console, the only real behaviour it has other than UI concerns is a single very simple calculation and a greater than check. Trying to "object-orient" that is much more likely to baffle than teach.

Understanding OO is a very important part in becoming a proficient Java programmer, but I'd say that you definitely took the right approach keeping this particular program procedural.

Answer 2

OO

I suggest an Object Oriented approach:

Make a CreditInfo class and the main part of your program will be just:

CreditInfo info = CreditInfo(
    positiveInput("Enter ...") ...
);
System.out.Println(info.calculateCredit());

The positiveInput function will allow for nicer input reading. You may put in a UserInterface class.

Answer 3

Structure

Right now, you just have all your code in the main method. This is not good for several reasons: It's hard to maintain, it's hard to test (you have to manually enter the data you want to test each time!), and it's hard to reuse part of the code.

So what would be a better approach? OOP certainly comes to mind. If you haven't looked into it yet, I would suggest to do that.

For such a small task, splitting the code into different methods might be enough though.

What you definitely want is to extract the code that does the actual calculation for each customer in such a way that it is independent of any input or output (ie it accepts the given facts as input, and returns true or false). This will make it easily reusable and also (automatically) testable.

Formatting

Your formatting is quite hard to read and not consistent. Your indentation is off, you have quite a lot of unnecessary vertical whitespace, etc.

Just paste your code into any IDE and let it format it for you.

Misc

• Declare variables as late as possible, instead of at the beginning of a method. This increases readability.
• Do not shorten variable names, as it decreases readability. A reader has to spent time thinking about what eg beg in begBalance means.
• Try to avoid duplication (eg the second print when wrong data was entered, or how integer input is read in general). You could for example write a method int getInteger(String promptText, String errorText) which is then used to get the begBalance, itemTotal, etc.

Answer 4

Welcome to CodeReview, please thoroughly read through the remarks. Feel free to post a follow-up question when you've implemented all the remarks you deemed be considered as overall positive feedback.

Some nit-picks:

Code Style

Right now the code is not very pleasant to read as you are very inconsistent with spaces and line breaks.

Magic Constants

(or -1 to quit) if I miss that line I might not understand the following either:

• if (accnum != -1)
• accnum != -1
• ...

Instead move it to a constant like private final static int QUIT_CODE = -1; and make use of that in the code, it makes it much more clear what the -1 represents, also makes it easier to change it in the future if you so decide.

Result

public class CreditCalc
{
    private final static int QUIT_CODE = -1;

    public static void main(String[] args) {
        Scanner input = new Scanner ( System.in )

        ...

        System.out.println ("Welcome to the Credit Calculator");
        System.out.println ("Please Enter Customer Account Number (or " + QUIT_CODE + " to quit): "); 
        accnum = input.nextInt ();
        if (accnum != QUIT_CODE) {

        ...

Also note I added spaced after method calls.

Object Orientation

The goal somewhere along the pipeline is to make sure that one .class is responsible for one thing. I would move credit card information to a separate class. Then just access is with getters/setters.