patternjavaMinor
Calculating and displaying number statistics
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).
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);
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!
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:
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
The point of most loop syntax is to make the exit conditions for the loop obvious. Instead of
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
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
This clarifies what you are comparing in a way that Magic Numbers could not:
Hopefully no-one would ever calculate percent as
UPDATE:
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
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.
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.