1
\$\begingroup\$

I'm a newcomer to Java (I primarily do C# ) and I have to say that they have made I/O handling much more difficult than it should have been. I am accustomed to the scenario where when I need keyboard input in a class of my application I simply call Console.Readline() and does the job. In Java, I must create a Scanner object in order to perform these operations and when input is needed in methods of different classes things get more complicated. So what I thought was creating an Input class where all keyboard inputs are handled there.

import java.time.* ;
import java.time.format.DateTimeParseException;
import java.util.Scanner;


public class Input
{
    private static Scanner scan = new Scanner(System.in);

    public static void UseEnterAsDelimiter()
    {
        scan.useDelimiter(System.getProperty("line.separator"));
    }

    public static int IntInRange(int min , int max, String InvalidMsg )
    {
        int value;

        while(true)
        {
            while (!scan.hasNextInt())
            {
                System.out.println("Expected an Integer. Please type again.");
                scan.next();
            }

            value = scan.nextInt();

            if( value >= min && value <= max)
                return value;
            else
            {
                System.out.println(InvalidMsg);
            }
        }
    }

    public static int Int()
    {
        int value;

        while (true)
        {
                while (!scan.hasNextInt())
                {
                    System.out.println("Expected an Integer. Please type again.");
                    scan.next();
                }

                value = scan.nextInt();

            return value;
        }
    }

    public static String String()
    {
        return scan.next();
    }

    public static String StringNoEmpty()
    {
        String value;

        while(true)
        {
            value = scan.next();
            if ( !value.isEmpty())
                return value;
            else
                System.out.println("Input cannot be blank. Please type again.");
        }

    }

    public static LocalDate Date()
    {
        LocalDate value;

        while(true)
        {
            try
            {
                String input = scan.next();
                if( input.isEmpty())
                    System.out.println("Field cannot be empty.");
                else
                {
                    value = LocalDate.parse(input);
                    return value;
                }
            }
            catch(DateTimeParseException e)
            {
                System.out.println("Date was not given in a correct format , please type it again.");
            }
        }
    }

    public static LocalTime Time()
    {
        LocalTime value;

        while(true)
        {
            try
            {
                String input = scan.next();
                if( input.isEmpty())
                    System.out.println("Field cannot be empty.");
                else
                {
                    value = LocalTime.parse(input);
                    return value;
                }
            }
            catch(DateTimeParseException e)
            {
                System.out.println("Time was not given in a correct format , please type it again.");
            }
        }
    }

    public static void CloseScanner()
    {
        scan.close();
    }


}

My question is if it's considered good practice to use a static Scanner and if it is what can I do to make this class better. I'm looking for improvement here, this class does what's expected but I don't know if it's the correct way.

\$\endgroup\$
3
\$\begingroup\$

My question is if it's considered good practice to use a static Scanner and if it is what can I do to make this class better. I'm looking for improvement here, this class does what's expected but I don't know if it's the correct way.

This depends on what you are building. This kind of static design might be ok for smaller utilities. For anything bigger it will come around to kick you sooner or later.

Your class is not really a class but a collection of utilities. Which again might be ok. Problem is with the Scanner which in your case is globally shared. Everybody who uses your class gets coupled to this global state.

Consider the below code:

public class BMI {
    public double value() {
        double weightInKg = (double) Input.Int();
        double heightInM = (double) Input.Int();
        Input.CloseScanner();
        return weightInKg  / (heightInM * heightInM);
    }
}

Since this uses globally shared state I just broke your code for all clients. Every piece of code depending on Input class will fail because I closed the scanner.

If I would now tried to write a unit test I would not be able to. Not without some dirty hacks. This code is coupled to Input and this dependency cannot be broken.

This is where Software Development principles such as SOLID come to into play.

Single Responsibility Principle:

Class should have a single reason to change. Your input class is responsible for everything regarding reading from console. That is a lot of reasons for change. From changing of date format to adding a new operations. Every such need causes change in this class and has potential to break something.

Open Closed Principle:

You want to be able to extend you program without touching existing code. If I wanted to read double for example I would have to change Input class. I should be able to just create snother class

Liskov Substitution Principle:

There is no substitution possible for Input. You cannot create other implementations of static utilities.

Interface Segregation Principle:

You want interfaces of your objects to be small so that object do not have to depend on things they do not use. Any class accessing Input has access to everything even though it just needs to read integers.

Dependency Inversion Principle:

Dependencies of your objects should be configurable. So that you can achieve easier reuse and testability. You cannot invert static dependency. It is there and you can't do anything about it.

There are many approaches how to make your code SOLID. Here is one example again with BMI class to show how client would use the code:

public class BMI {

    private final DoubleProvider weightInKg;
    private final DoubleProvider heightInM;

    public BMI(DoubleProvider weightInKg, DoubleProvider heightInM) {
        this.weightInKg = weightInKg;
        this.heightInM = heightInM;
    }

    public double value() {
        final double weight = weightInKg.doubleValue();
        final double height = heightInM.doubleValue();
        return weight  / (height * height);
    }
}

public class Input implements DoubleProvider, IntProvider {

    private final Scanner scanner;

    public Input(Scanner scanner) {
        this.scanner = scanner;
    }

    @Override
    public double doubleValue() {
        ...
    }

    @Override
    public int intValue() {
        ...
    }

    ...

}

Most notable change are the dependencies. My BMI class no longer depends on details of Input. It can read from console, file, network... anything as long as it implements DoubleProvider interface.

Nothing prevents me from testing both BMI and Input. Dependencies are configurable via constructor.

\$\endgroup\$
  • \$\begingroup\$ Your example made me understand a lot of things on how Java works, thanks! \$\endgroup\$ – NickDelta Apr 25 at 18:00
  • \$\begingroup\$ @NickDelta Almost nothing what Januson wrote has to do with Java, it all applies to C# (or almost any other language) in the same way. \$\endgroup\$ – RoToRa Apr 26 at 8:10
  • 1
    \$\begingroup\$ @RoToRa Yes, I'm aware of that,I was just convinced that I had to close the Scanner but eventually if I do that anywhere in the project, all other Scanners will fail after that. That's why I said it. \$\endgroup\$ – NickDelta Apr 26 at 8:47
3
\$\begingroup\$

Since it always operates on System.in, which is static, it's fine to have a static class. You could name it "SystemInput" to make the purpose clearer and make the class final with private constructor to make a point that it's not to be instantiated.

Your naming convention is uncommon to Java. Methods should start ith lower case letter.

The class is essentially just a big chunk of utility methods. To maintain the single responsibility principle, you could split them into their own classes that operate on a given Scanner. Maybe implement a common typed interface... You're just doing procedural programming with a global variable now.

Using the wildcard in imports is a bit bad practise. It becomes hard to tell what you actually need. It's also a sign that your class might have too many responsibilities if you have to resort to "just gimme everything".

What kind of projects do you usually do? (Ok, this is purely anecdotal, but) I've been programming Java for about 20 years and while I've parsed command line parameters quite often and read data piped from other commands I don't remember ever reading input from console interactively after leaving the university "Introduction to Java" course.

\$\endgroup\$
  • 1
    \$\begingroup\$ Well I'm a 1st year undergraduate. I say that I'm primarily doing C# cause I've been in touch with it since I was 13 and during these 6 years I've advanced in a point where I'm very fluent with it. But let's go to your answer : I'm aware of the single responsibility principle, but I don't understand why I'm violating it. All class methods are basically used for user input. I'm assuming that you mean that the class handles many Data Types (int, String etc.). I would do the interfaces if was writing for enterprise, wouldn't be difficult but we haven't even done it in class. \$\endgroup\$ – NickDelta Apr 25 at 12:09
  • \$\begingroup\$ Regarding the imports, if you're using eclipse as your IDE, you can press ctrl-shift-O to organize imports which will remove unused imports and only show the actual used ones. \$\endgroup\$ – Bill Hileman Apr 25 at 15:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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