Glen Peterson's advice is good, but there's something I'd like to add:
You're programming in Java, an object-oriented language. When first learning to program, it's often hard to imagine the practical value of using objects. Therefore, as a beginner one often ends up with a style similar to yours: all code is in a single class, and indeed within a single method in that class.
This style is called monolithic and although it has the benefit of being very concise, it leads to unmaintainable, hard-to-debug and unreadable code. As the requirements evolve in complexity, these problems become worse.
To solve them, first analyse the responsibilities of your code:
- Input/Output
- Validation
- Tracking statistics
These concerns should be handled by collaborating objects. For instance, to track statistics, you could create a StatisticsTracker:
public class StatisticsTracker { private int count; private int total; private int minimum; private int maximum; public void addNumber(int number) { count++; total += number; adjustMinimumAndMaximum(number); } private void adjustMinimumAndMaximum(int number) { if (containsSingleNumber()) { minimum = number; maximum = number; } else if (number < minimum) { minimum = number; } else if (number > maximum) { maximum = number; } } private boolean containsSingleNumber() { return count == 1; } public int getCount() { return count; } public int getTotal() { return total; } public int getMinimum() { return minimum; } public int getMaximum() { return maximum; } public double getAverage() { return (double) total / count; } }
Notice that you don't need to hang on to an array or list of numbers; you can track the statistics as you add numbers. Please also note that all this code knows nothing about validation or input/output: all it cares about is tracking statistics.
Next, we'll need a collaborator that validates our input. Because I'm assuming your validation requirements are likely to change, it makes sense to define an interface (T means there can be validators for any type of data, such as strings, integers or floating-point numbers):
public interface Validator<T> { public ValidationResult validate(T input); }
Our validator needs to check if the input is between two bounds, so let's implement the interface:
public class IntegerBoundsValidator implements Validator<Integer> { private int lower; private int upper; public IntegerBoundsValidator(int lowerBound, int upperBound) { lower = lowerBound; upper = upperBound; } @Override public ValidationResult validate(Integer input) { if (input < lower) { return ValidationResult.invalid("input must be greater than " + lower); } if (input > upper) { return ValidationResult.invalid("input must be smaller than " + upper); } return ValidationResult.valid(); } }
You'll notice the use of a class called ValidationResult. This is a Value Type. It has no significant behaviour; it just contains data and some static convenience methods to create instances of it, as seen below. Some OOP purists disagree with my use of public final fields; feel free to make them private and add getters if you so desire.
public class ValidationResult { public final boolean isValid; public final String message; private static final ValidationResult VALID = new ValidationResult(true, ""); public static ValidationResult valid() { return VALID; } public static ValidationResult invalid(String message) { return new ValidationResult(false, message); } private ValidationResult(boolean isValid, String message) { this.isValid = isValid; this.message = message; } }
To connect all these parts together, we'll use a class called Main - for the lack of a better name.
public class Main { private static final Scanner input = new Scanner(System.in); private static final Validator<Integer> validator = new IntegerBoundsValidator(0, 100); private static final StatisticsTracker statistics = new StatisticsTracker(); public static void main(String[] args) { while (input.hasNextInt()) { int next = input.nextInt(); handleInput(next); } printStatistics(); } public static void handleInput(int input) { ValidationResult result = validator.validate(input); if (result.isValid) { statistics.addNumber(input); } else { printLine(result.message); } } private static void printStatistics() { if (statistics.getCount() == 0){ printLine("No valid input."); return; } printLine("Total: %d", statistics.getTotal()); printLine("Count: %d", statistics.getCount()); printLine("Min: %d", statistics.getMinimum()); printLine("Max: %d", statistics.getMaximum()); printLine("Average: %.2f", statistics.getAverage()); } private static void printLine(String format, Object... args) { System.out.println(String.format(format, args)); } }
Conclusion
Why go to all this effort of refactoring? Well, the benefits are apparent:
- Each class now fulfils a clearly-defined responsibility. Changes in one of those classes will not affect the other responsibilities.
- Each method within these classes is short and has a readable name. These methods can and should be tested separately with unit tests, which is close to impossible with monolithic code.
- The system is extensible: It's easy to change your validation requirements by simply plugging in a different
Validator instance. You could create a CompositeValidator that combines several validators into a single validator chain.
Of course, you may disagree with some of the implementation details — but this review isn't really about those. It's more of a demonstration of which questions you should be asking in order to end up with clean code.
Good luck with your learning, and thanks for sharing your code.
