Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite
1

I started learning java a few days ago. I prefer C++ but since I'll need java this college semester I decided to learn a few things to ease myself in. Anyway in C++ I always used a foolproof input method for taking user input of primitive numeric types. I picked that method up from here.

So I decided to somewhat transfer that to Java with perhaps an added functionality or two. However it turned out to be quite a bit messier than what I thought.

This is my current code:

FoolproofInput.java

package mrplow.input;

import java.util.Scanner;

/*
----------------------------------------------------------------------
@by MrPlow

Class designed for bullet-proofing user input for Integer, Float and 
Double primitive types

They check if the next item in the input stream is valid:
- if it is, the read value is returned
- if it is not, the prompt loops until the correct item is input

All of the methods are used in the same way:
ex.

myInt = FoolproofInput.readInt("Give me an int");

There are also read<T>Rng methods
They are used for range input and contain two additional parameters 
which are the lower range bound "lowest" and upper range bound "highest"

ex.

myInt = FoolproofInput.readIntRng("Give me an int between 10 - 20", 11,19);
------------------------------------------------------------------------
*/
public class FoolproofInput
{
    static private Scanner scan = new Scanner(System.in);
    static private boolean isValid = false;
    static private int intValue = 0;
    static private float floatValue = 0;
    static private double doubleValue = 0;

    static public int readInt(String prompt)
    {
        intValue = 0;

        do
        {
            System.out.print(prompt);
            if(scan.hasNextInt())
            {
                intValue = scan.nextInt();
                isValid = true;
                scan.nextLine();
            }
            else
            {
                System.out.println("Invalid input! Requested an Int, recieved something else.");
            isValid = false;
            scan.nextLine();
        }
    }while(!isValid );

    return intValue;
    }

    static public float readFloat(String prompt)
    {
        floatValue = 0;

        do
        {
            System.out.print(prompt);
            if(scan.hasNextFloat())
            {
                floatValue = scan.nextFloat();
                isValid = true;
                scan.nextLine();
            }
            else
            {
                System.out.println("Invalid input! Requested a Float, recieved something else.");
                isValid = false;
                scan.nextLine();
            }
        }while(!isValid );

        return floatValue;
    }

    static public double readDouble(String prompt)
    {
        doubleValue = 0;

        do
        {
            System.out.print(prompt);
            if(scan.hasNextDouble())
            {
                doubleValue = scan.nextDouble();
                isValid = true;
                scan.nextLine();
            }
            else
            {
                System.out.println("Invalid input! Requested a Double, recieved something else.");
                    isValid = false;
                    scan.nextLine();
                }
            }while(!isValid );

        return doubleValue;
    }

    static public int readIntRng(String prompt, int lowest, int highest)
    {
        int intValue = 0;

        do
        {
            System.out.print(prompt);
            if(!scan.hasNextInt())
            {
                System.out.println("Invalid input! Requested an Int, recieved something else.");
                isValid = false;
                scan.nextLine();
            }
            else
            {
                intValue = scan.nextInt();
                if( intValue >= lowest && intValue <= highest )
                {
                    isValid = true;
                    scan.nextLine();
                }
                else
                {
                    System.out.println( "Invalid input! The input number is not in range:"
                                        + "(" + lowest + "-" + highest + ")." );
                    intValue = 0;
                }
            }
        }while(!isValid );

    return intValue;
    }

    static public float readFloatRng( String prompt, float lowest, float highest )
    {
        float floatValue = 0;

        do
        {
            System.out.print(prompt);
            if(!scan.hasNextFloat())
            {
                System.out.println("Invalid input! Requested a Float, recieved something else.");
                isValid = false;
                scan.nextLine();
            }
            else
            {
                floatValue = scan.nextFloat();
                if(floatValue >= lowest && floatValue <= highest)
                {
                    isValid = true;
                    scan.nextLine();
                }
                else
                {
                    System.out.println( "Invalid input! The input number is not in range:"
                                        + "(" + lowest + "-" + highest + ")." );
                    floatValue = 0;
                }
            }
        }while(!isValid );

    return floatValue;
    }

    static public double readDoubleRng( String prompt, double lowest, double highest )
    {
        double doubleValue = 0;

        do
        {
            System.out.print(prompt);
            if(!scan.hasNextDouble())
            {
                System.out.println("Invalid input! Requested a Double, recieved something else.");
                isValid = false;
                scan.nextLine();
            }
            else
            {
                doubleValue = scan.nextDouble();
                if(doubleValue >= lowest && doubleValue <= highest)
                {
                    isValid = true;
                    scan.nextLine();
                }
                else
                {
                    System.out.println( "Invalid input! The input number is not in range:"
                                        + "(" + lowest + "-" + highest + ")." );
                    doubleValue = 0;
                }
            }
        }while(!isValid );

    return doubleValue;
    }
}

Basically this has too much repeat code and the only differences are the (hasNext() methods).

So is there any way this code can be shortened? And are there any possible bugs that I've missed?

P.S: Also why doesn't this place have a "user-input" or even "input" tag?

share|improve this question

1 Answer

up vote 4 down vote accepted

Among other things, you're relying on 'global' (class-level) variables way too much. This will cause you problems later, especially if you want to do any sort of multi-threaded programming. You also haven't separated your concerns as much as you probably should - actually doing so may help make it easier to test things.

Java makes heavy use of classes and interfaces, which is what you really want here (potentially including anonymous classes). Here's what you'll need:

1) An interface for your input: 

package myplow.input;

import java.util.Scanner;

public interface InputGrabber<T> {

    boolean hasNextInput(Scanner scanner);

    T getNextInput(Scanner scanner);

    String getExpectedInputFormat();

}

2) Something to control the input cycle:

package myplow.input;

import java.io.InputStream;
import java.io.PrintStream;
import java.util.Scanner;

public class PromptCycle {

    private final Scanner scanner;
    private final PrintStream output;

    public PromptCycle(InputStream in, PrintStream out) {
        scanner = new Scanner(in);
        output = out;
    }

    public <T> T getInput(String prompt, InputGrabber<T> grabber) {
        do {
            output.println(prompt);
        } while (needsProperInput(grabber));
        T value = grabber.getNextInput(scanner);
        return value;
    }

    private <T> boolean needsProperInput(InputGrabber<T> grabber) {
        if (grabber.hasNextInput(scanner)) {
            return false;
        }
        output.println(grabber.getExpectedInputFormat());
        scanner.nextLine();
        return true;
    }

}

3) Implementations of the interface to allow validation/retrieval:

package myplow.input;

import java.util.Scanner;

public class IntegerInputGrabber implements InputGrabber<Integer> {

    public boolean hasNextInput(Scanner scanner) {
        return scanner.hasNextInt();
    }

    public Integer getNextInput(Scanner scanner) {
        return scanner.nextInt();
    }

    public String getExpectedInputFormat() {
        return "Invalid input! Requested an Int, recieved something else.";
    }

}

4) You can then 'wire' up an application like this:

package myplow.input;

public class Application {

    /**
     * @param args
     */
    public static void main(String[] args) {
        System.out.println("Application started");
        PromptCycle cycle = new PromptCycle(System.in, System.out);
        Integer value = cycle.getInput("Needs an int", new IntegerInputGrabber());
        System.out.println("Value retrieved: " + value);
        System.out.println("Application ended");
    }

}

This has the benefits of not caring where the actual 'scanned' input comes from (ie it's trivial for me to use something other than standard input/output). Additionally, I can add whatever sort of input grabber I want - including non-primitives - simply by passing in a different implementation. The getInput() method is genericized, so the return type is based on whatever gets passed in.

All code compiles and runs as expected. However, there should probably be better checks than what I have so far. The rest of the input grabber implementations I'm leaving as an exercise for the reader.

I'm also including one for a range, because it's somewhat non-intuitive, given the way I have the cycles set up:

package myplow.input;

import java.util.Scanner;

public class IntegerRangeInputGrabber implements InputGrabber<Integer> {

    private final int minimum;
    private final int maximum;

    public IntegerRangeInputGrabber(int minimum, int maximum) {
        this.maximum = Math.max(maximum, minimum);
        this.minimum = Math.min(maximum, minimum);
    }

    public boolean hasNextInput(Scanner scanner) {
        if (!scanner.hasNextInt()) {
            return false;
        }
        int value = scanner.nextInt();
        return (value >= minimum && value < maximum);
    }

    public Integer getNextInput(Scanner scanner) {
        return Integer.valueOf(scanner.match().group());
    }

    public String getExpectedInputFormat() {
        return "Invalid input! The input number is not in range:" + 
                     "(" + minimum + "-" + maximum + ").";
    }

}

This can of course be wired up similarly to the other grabbers.

share|improve this answer
This will certainly help me in learning more about Java, thanks for taking your time to help me. One question though. How is private <T> boolean needsProperInput(InputGrabber<T> grabber) valid? Aren't you declaring multiple return types in one method? If T is Int wouldn't it be int boolean or does boolean work differently in Java than in C++? – MrPlow Oct 2 '12 at 12:01
The <T> before the return-type definition is a type-parameter. That's what's allowing me to use the generic form of InputGrabber in the method parameter. It's not considered a 'multiple return type' (it's not returned, and the method could actually have a void return type), it's basically a compile-time-check parameter. Have a look at Java's generics for more information. – Clockwork-Muse Oct 2 '12 at 15:38

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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