HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Calculating and displaying number statistics

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numberdisplayingcalculatingstatisticsand

Problem

I'm writing a program which takes individual int inputs from the user and then outputs some details on the numbers (average, min, max and how many).
Input.readInt() is a method I made which simply reads an integer from the command line and returns it.

There are a few things I'd like to improve - for the average the float does not seem to work, it only holds prints out value to nearest integer. I'd also like to format this to always display 2 decimal places.
Is there any way to dynamically assign the size of the array? So I could enter more than 10 values?

I'm looking for advice on more efficient methods (both speed and code length), and any other general advice which could help me learn.

```
public class NumberStats {

public static void main(String[] args) {
int[] input = new int[10];
int i;
int total = 0;
int min = 0;
int max = 0;
double average = 0;
int count = 0;

while (true) { // READ INPUTS
System.out.print("Please enter a number: ");
input[count] = Input.readInt();

if (input[count] == 0) {
break;
} else if (input[count] 100) {
System.out
.println("Warning: "
+ input[count]
+ " is not valid and will be excluded from final statistics.");
}

if (input[count] max) {
max = input[count];
}
count++;
System.out.println("#" + count + ": " + input[count - 1]);
}

}
if (count != 0) {
for (i = 0; i < input.length; i++) { // CALCULATE SUM
total += input[i];
}
average = total / count; // CALCULATE AVERAGE

System.out.println("Total: " + total);
System.out.println("Count: " + count);
System.out.println("Min: " + min);

Solution

Please tell me this is for your personal benefit and that you aren't asking me to do your homework!

average = total / count;


total and count are both integers, so / performs integer division. The result is then converted to a double. This should fix your problem:

average = ((double) total) / ((double) count);



Is there any way to dynamically assign the size of the array? so i
could if i wish enter more than 10 values?

Use a java.util.ArrayList:

// old:
// int[] input = new int[10];
// new:
List input = new ArrayList();



I'd also like to format this to always display 2 decimal places

Look at java.text.DecimalFormat.

General Advice

You do an awful lot of typing and a lot of dereferencing (and typing!). Declare a local inVal parameter:

int inVal = 0;
...

inVal = Input.readInt();
if (inVal < 0) {
    ...
}
// Finally, after all your checks and calculations, add inVal to your array (or List):
input[count] = inVal;


The point of most loop syntax is to make the exit conditions for the loop obvious. Instead of while (true) I'd like to see something like:

do {
    ...
} while (inVal >= 0)


I like that you calculate your max and min in the input loop. You could count your total in that loop as well instead of taking a second loop through your data. Hmm... If you do that, you don't need to use an array or a List, just inVal.

You have "magic numbers" 0 and 100. Years from now when three other programmers have worked on this code, they may add a percentage calculation as x / 100 so that when you decide to increase your number of inputs to 200, you search-and-replace 100 with 200 you have broken your percentage calculation. You should factor these out of your code:

public final int MAX_NUM_INPUTS = 100;
public final int INPUT_TOO_LOW_VALUE = 0;


This clarifies what you are comparing in a way that Magic Numbers could not:

if (count > MAX_NUM_INPUTS) ...

if (inVal <= INPUT_TOO_LOW_VALUE) ...


Hopefully no-one would ever calculate percent as x / MAX_NUM_INPUTS. Now you can change these limits in one place without doing any search-and-replace.

UPDATE:

System.out.println...

Yeah, Java is funny about string concatenation. If you have a non-final variable as part of a String, it can cause a bunch of nasty overhead. For a little program like yours, it really doesn't matter what you do. If it were going to print out a String in a loop, then I'd suggest the following change. Note that System.err.println() uses an OS-specific line separator - not necessarily \n.

// This is fine for your program..
System.err.println("Error: " + input[count]
                        + " is not valid, exiting.\n");

// Option 1: Use multiple print stmts to avoid String concatenation:
System.err.print("Error: ");
System.err.print(inVal);
System.err.println(" is not valid; exiting.");
System.err.println();

// Option 2: Use a StringBuilder (also uses "method chaining" idiom):
System.err.println(new StringBuilder().append("Error: ")
                                      .append(inVal)
                                      .append(" is not valid; exiting.")
                                      .toString());
System.err.println();


This really isn't important in your specific example because it's so small. In a bigger program or a production situation String concatenation can be a real performance problem. People often make mistakes in logging code that really slows a system down. I think later versions of Java have tamed this issue. I know IntelliJ IDEA reports various times when it's better to use the + signs vs. a StringBuilder. I've checked the compiled code a few times when I thought it was wrong, and it turned out to be right, so now I just trust it to tell me.

Code Snippets

average = total / count;
average = ((double) total) / ((double) count);
// old:
// int[] input = new int[10];
// new:
List<Integer> input = new ArrayList<Integer>();
int inVal = 0;
...

inVal = Input.readInt();
if (inVal < 0) {
    ...
}
// Finally, after all your checks and calculations, add inVal to your array (or List):
input[count] = inVal;
do {
    ...
} while (inVal >= 0)

Context

StackExchange Code Review Q#21383, answer score: 5

Revisions (0)

No revisions yet.