Thread: Help me improve my code.

Hi. I'm still studying my programming and am looking for ways to keep my code more readable and less redundant. This is my latest exercise. It runs and executes the way I want it to, but I have a sneaking suspicion that the code can be written a little better. Any help will be greatly appreciated. Thanks.

~Jeremiah

 
//Jeremiah A. Walker
//Class declaration with one method
//Write an application that reads a product number and the quantity of the product sold
import java.util.*; //scanner
 
public class CalSales 
{
	private String courseName;//course name for this quantitybook
	//int instance variables are initialized to 0 by default
	private int total;//sum of quantitys
	private int pCounter;// number of quantitys entered
	private int pCount;//choice variable for user
	private int p1Count;//number a As
        private int p2Count;//number a Bs
        private int p3Count;//number a Cs
        private int p4Count;//number a Ds
        private int p5Count;//number a Fs
        private double price1 = 2.98;//price of product 1
        private double price2 = 4.50;//price of product 2
        private double price3 = 9.98;//price of product 3
        private double price4 = 4.49;//price of product 4
        private double price5 = 6.87;//price of product 5
	private	double pret1 = 0; 
	private	double pret2 = 0;
	private	double pret3 = 0;
	private	double pret4 = 0;
	private	double pret5 = 0;
 
 
	//constructor initializes courseName with String argument
	public CalSales ( String name  )// constructor name is class name
	{
		courseName = name; //initializes courseName
	}//end counstructor
 
 
	//method to set the course name
	public void setCourseName ( String name )
	{
		courseName = name; //store the course name
	}// end method setCourseName
 
	//method to retrieve the course name
	public String getCourseName()
	{
		return courseName;
	}// end method getCourseName
 
 
	//display a welcome message to the quantitybook user
	public void displayMessage()
	{
		System.out.print("Welcome to the quantity Book for: \n" + getCourseName() + "!\n" );
	}//end method displayMessage
 
	public void inputquantitys()
	{
		Scanner input = new Scanner(System.in);
		int quantity = 0; //quantity input by user
 
 
 
		System.out.print("Enter the product number of each good sold(1-5), then enter the quantity.  \n Type an end-of-file indicator to terminate input:\n On Unix/Linux/Mac OSX type CTRL-d and enter.   On Windows, type CTRL-z and enter");
		//loop until user enters end-of-file indicator
		while(input.hasNext())
		{
			pCount = input.nextInt(); // read product number
			System.out.print("Entered product = " + pCount + "\n");
			++pCounter;
			//call method to increment appropriate counter
			incrementLetterquantityCounter(quantity);
		}//end while
	}//end input quantitys
 
	private void incrementLetterquantityCounter(int quantity)
	{
		Scanner input = new Scanner(System.in);
		//determines letter value of quantity entered
		switch (pCount)
		{
			case 1://if product number is 5
				System.out.print("Price = " + price1 + ". Enter quantity sold for Product 1:  ");
				quantity = input.nextInt();
				p1Count += quantity;//add quantity entered to pcount
				total += quantity;
				pret1 = price1 * p1Count;
                        	System.out.print("$" + pret1 + "\n");
				break;//break to exit switch
 
			case 2://if product number is 4
				System.out.print("Price = " + price2 + ". Enter quantity sold for Product 2:  ");
				quantity = input.nextInt();
				p2Count += quantity;
				total += quantity;
				pret2 = price2 * p2Count;
                        	System.out.print("$" + pret2 + "\n");
				break;//break to exit switch
 
			case 3://if product number is 3
				++p3Count;//increment aCoun
				System.out.print("Price = " + price3 + ". Enter quantity sold for Product 3:  ");
				quantity = input.nextInt();
				p3Count += quantity;
				total += quantity;
				pret3 = price3 * p3Count;
                        	System.out.print("$" + pret3 + "\n");
				break;//break to exit switch
 
 
			case 4: //if product number is 2
				++p4Count;//increment aCoun
				System.out.print("Price = " + price4 + ". Enter quantity sold for Product 4:  ");
				quantity = input.nextInt();
				p4Count += quantity;
				total += quantity;
				pret4 = price4 * p4Count;
                        	System.out.print("$" + pret4 + "\n");
				break;//break to exit switch
 
			case 5://if product number is 1
				++p5Count;//increment aCoun
				System.out.print("Price = " + price5 + ". Enter quantity sold for Product 5:  ");
				quantity = input.nextInt();
				p5Count += quantity;
				total += quantity;
				pret5 = price5 * p5Count;
                        	System.out.print("$" + pret5 + "\n");
				break;//break to exit switch
			default: //if the quantity was less than 60--if no other condition is true,--, the quantity is an 0
			System.out.print("Unknown product number entered");
				break;//exit the switch--optional
		}//end switch
	}//end incrementLetterquantityCounter
 
	public void displayquantityReport()
	{
		System.out.println("\nSales Report:");
		//if the user entered at least one quantity (IE quantity is not zero)...
		if(pCounter != 0)
		{
			//calculate average of all quantitys entered
			double average = (double) total/pCounter;
 
 
			//output summary of results
			System.out.print(pCounter + " sales were entered.\n");
			System.out.print("Total  = " + total + "\n");
			System.out.print("Average  = " + average + ".\n");
			System.out.print("P1: " + p1Count + "\n" + pret1 + "\n");
			System.out.print("P2: " + p2Count + "\n" + pret2 + "\n");
			System.out.print("P3: " + p3Count + "\n" + pret3 + "\n");
			System.out.print("P4: " + p4Count + "\n" + pret4 + "\n");
			System.out.print("P5: " + p5Count + "\n" + pret5 + "\n");
		}//end if
		else //if no quantitys were entered, output a message indicating so.
			System.out.println("No quantitys were entered.\n");
	}//end displayquantityReport
 
}//end class quantitybook

and the tester

//Jeremiah A. Walker
//Creating a  quantitybook object and calling its displayMessage method.
 
 
public class CalSalesTest 
{
	//main method begins program execution
	public static void main( String[] args )
	{
		//create quantitybook object
		CalSales CalSales1 = new CalSales("Bill Dauterive");
 
		//display welcome message
		CalSales1.displayMessage();//welcome message
		CalSales1.inputquantitys();//input quantitys from user
		CalSales1.displayquantityReport();//display results
 
	}//end main
}// end class quantitybooktest

Well, you should call the method setCourseName(name) rather than just using the code for it in the constructor, repeating it in the setCourseName() but never using the method setCourseName().

Thanks and how would you write that?

On the readability front there are a few things you could do. (Most advice you'll get in this category comes with the label "my 2c". You may think it's overvalued...)

Use Java coding conventions and start variables and methods with a lowercase letter. calSales1 etc.

Remove comments that add no value to the code. "end method setCourseName" and the like are just noise and obscure rather than explain.

Keep comments accurate. So, correct (or remove) "case 2://if product number is 4".

Consider javadoc style comments. (and run javadoc on your code to generate the html comments - it's quite satisfying.)

    /** Creates a new set of sales data with a given course name. */
public CalSales ( String name  )
{
    courseName = name;
}

Originally Posted by javapenguin

Well, you should call the method setCourseName(name) rather than just using the code for it in the constructor

You ought to be a little careful if you call methods in the constructor. If they can be overridden by subclasses (ie are not marked final): there could be surprises.

Thanks a lot. I'll keep practicing

Thanks a lot. I'll keep practicing