8
\$\begingroup\$

Is it a good practice to initialize the max to -100000 and min to 100000? Is there any other way to initialize both min and max to 0?

 import javax.swing.*;
    import java.util.*;

public class arrayTajba
{
    public static void main(String[] args) throws Exception
    {
        String userStringInput = " ";
        String display = " ";
        int max = -100000;
        int min = 100000;
        int total = 0;
        double average;
        int i = 0;
        int [] num = new int [5];

        for (i = 0; i < num.length; i++)
        {
            userStringInput = JOptionPane.showInputDialog(null,"Please enter 5 numbers " + (i+1),"Input Table",JOptionPane.QUESTION_MESSAGE);
            num[i] = Integer.parseInt(userStringInput);

            display+=num[i] + ", ";
            total+=num[i];

            if (num[i] < min)
            {
                min = num[i];
            }
            if (num[i] > max)
            {
                max = num[i];
            }
        }
            average = total / num.length;

        JOptionPane.showMessageDialog(null,"The numbers you have entered are:\n" + display + "\nSum of all numbers is: " + total + "\nAverage is: " + average + "\nMinimum number is: " + min + "\nMaximum number is: " + max,"Output Table" ,JOptionPane.INFORMATION_MESSAGE);
        }
    }i
\$\endgroup\$

2 Answers 2

12
\$\begingroup\$

Java has a couple of constants available that are commonly used in situations like this:

    int max = Integer.MIN_VALUE;
    int min = Integer.MAX_VALUE;

Using these two values ensures you cover the possible spectrum of valid input values.

In addition, a sadistic user would enter 5 large values, and cause your total value to overflow, creating a bad result. You should declare your total as a long value:

    long total = 0;

That will prevent overflow (unless you have 2 billion input values.... ;-)

Finally, the last statement is a problem, it does integer division:

        average = total / num.length;

It should convert one of the values to a double, so that floating-point division happens instead:

        average = total / (double)num.length;
\$\endgroup\$
6
  • 1
    \$\begingroup\$ I'd actually just initialize both to num[0] as you just need to check for the max/min inside the input-array.. \$\endgroup\$
    – Vogel612
    Jun 23, 2014 at 15:59
  • \$\begingroup\$ @Vogel612 - that requires putting in a special case for if (i == 0) {....} inside the loop. \$\endgroup\$
    – rolfl
    Jun 23, 2014 at 16:01
  • \$\begingroup\$ @Vogel612 How would you do that as the numbers receive their value inside the loop? \$\endgroup\$ Jun 23, 2014 at 16:06
  • \$\begingroup\$ @Cruncher - that would make a decent answer, you should add one. \$\endgroup\$
    – rolfl
    Jun 23, 2014 at 20:41
  • \$\begingroup\$ @Vogel612 You are absolutely correct. In many (in fact, all) situations initializing min and max to the first available value is the only way to go. \$\endgroup\$
    – vnp
    Jun 24, 2014 at 8:11
5
\$\begingroup\$

As mentioned in a comment, I think your whole process smells. You should check for the Single Resposibility Principle (SRP). In essence the principle says:

A method, class or package should only be responsible for exactly one thing. Methods do one task, classes represent one specific object and packages organize one part of your application.

Now if we look at your code and see what the main method does, we see it is responsible for:

reading input, "keeping" it, calculating average, max and min, outputting results

Now let's split that up into methods, that are responsible for a single thing. I suggest you keep the data in the main-method.

Other minor and major things:

String concatenation and formatting:

 "The numbers you have entered are:\n" + display + "\nSum of all numbers is: " + total + "\nAverage is: " + average + "\nMinimum number is: " + min + "\nMaximum number is: " + max

This is the complicated way to do things. More -like would be to use the String.format-method:

String.format("The numbers you have entered are:\n %s" 
    + "\n Sum of all numbers is: %d\nAverage is: %d" 
    + "\nMinimum number is: %d\nMaximum number is:%d",
    numbers.toString(), total, average, min, max);

Reusing the analysis result:

It's difficult to pass around the analysis result if you'd need it in some other place, than as Strings in your output. Thus I'd recommend using an inner class to store the result. In that class you should also do the String operations and stuff.

If you need the results somewhere else, you can easily make the inner class a "public" one and move it out of your ArrayTajba. Then you can use it "anywhere" ;)

Magic numbers:

5 is a magic number in your code. What if you want to have 7 numbers instead? I suggest extracting it to a named constant.

Rewriting your code:

public class ArrayTajba {
    private static final int NUMBER_COUNT = 5;

    public static void main(String[] args) {
        int[] numbers = readUserInput(NUMBER_COUNT);
        AnalysisResult results = AnalysisResult.analyze(numbers);
        outputToUser(results.toString());
    }

    private static int[] readUserInput(final int inputCount) {
       int[] results = new int[inputCount];

       for(int i = 0; i < results.length; i++){
           String userStringInput = JOptionPane.showInputDialog(/* shortcutting here */);
           results[i] = Integer.parseInt(userStringInput);
        }
        return results;
    }

    private static void outputToUser(final String message){
       JOptionPane.showMessageDialog(null, message, JOptionPane.INFORMATION_MESSAGE);
    }

    private final class AnalysisResult {
        private int[] numbers;
        private int max;
        private int min;
        private int total;
        private double average;

        public static AnalysisResult analyze(final int[] numbers) {
            AnalysisResult result = new AnalysisResult(numbers);
            result.runCalculations();
            return result;
        }

        private AnalysisResult(final int[] inputArray) {
            //prevent changes from outside
            numbers = inputArray.clone();
        }

        private void runCalculations() {
            min = numbers[0];
            max = numbers[0];
            total = 0;

            for (int number : numbers) {
                if(number < min) {
                    min = number;
                }
                if(number > max) {
                    max = number;
                }
                total += number;
             }
             average = total / (double) number.length;
        }

        public String toString() {
            return String.format("The numbers you have entered are:\n %s" 
                       + "\n Sum of all numbers is: %d\nAverage is: %d" 
                       + "\nMinimum number is: %d\nMaximum number is:%d",
                       numbers.toString(), total, average, min, max);
        }
    }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.