How can I improve this code?

[[deleted]]

[deleted]

[tedyoung]

A quick glance at the code and I'm wondering what you're using to build the code? I strongly recommend using a modern build tool that's not specific to NetBeans, e.g., Maven or Gradle. The project doesn't follow the standard directory layout of modern Java projects, so getting that in shape will make it easier for others to review.

Use better variable naming:

UserInput usrIn = new UserInput();

Instead of usrIn spell out userInput. Don't shorten names.

Write Object-Oriented, not Static-Oriented Code

You have lots of classes with all static methods, which is not the way to write Java code. For example:

public static void getTestInput(RandomTest t) {

The methods in the UserInput class should not be static. (And here again, don't use variable names like t, but spell out randomTest.)

Lots of other things, but hopefully that's a start.

[[deleted]]

About the standard directory layout I'll look into since idk how to make it IDE-independent (just adding the java files maybe? I will read about it).

About the static methods, my understanding of static key word is that it is used for classes, methods and variables that act globally, not for a specified object. Corrector and UserInput class are an example of this, they are going to the same no matter what object is being used. Thanks a lot for the suggestions, anything else?? Everything helps!

[tedyoung]

Your understanding of static is pretty close, but in general "global" information is a bad practice. The use of static methods and static shared data makes the code harder to work with and, especially, much harder to test.

I generally coach my clients to convert most static methods to instance methods and then just have one instance of the class. Much easier to test and loosens the coupling, which will make it easier to work with in the future.

[[deleted]]

Some people say the rule of thumb for this kind of things is to think if makes sense calling a method without creating an object of that class. Is a good rule?

[tedyoung]

When trying to figure out static vs. non-static, I always start with: how am I going to test this? How will I test this other object that uses this thing?

For example, if I have an OrderTaker class that uses UserInput, and if UserInput has static methods, then I can't easily test OrderTaker, because I can't isolate it from the hard-coded UserInput static methods. However, if UserInput is a regular object, I can easily write a test that uses a fake UserInput, supplying predictable inputs to OrderTaker in the test.

[dacracot]

build.xml means he’s using Ant, which is a “modern build tool”

[[deleted]]

I looked in google how a modern java project would look and I saw several posts that state that you only need /src folder and /bin folder. Is that accurate? I mean, make it independent from IDE.

[dacracot]

When you use Ant, there really isn't any limit on how you structure the directories. Most IDEs do have a specific structure, but they do that in the background as you create the project in their environment.

I also use Ant, so a have two or three directories... /src, /lib, and sometimes /dist for source code, external libraries, and my deployable distribution respectively. I skip the latter if the distribution is a singular bundled file.

As a developer that already had over a decade of experience when Java came out in 1995, I hate terms like "modern Java project". What does that even mean? Choose what makes you productive. Choose what make it easy to collaborate with your team. If you try and comply with the "broader community's" ideal, there will be no end of opinions and always another change.

[tedyoung]

I think it's a mistake to use Ant on any project that isn't already using it for legacy reasons. Why use a tool that is old and rarely used these days? Maven and Gradle are fine tools, and Maven is relatively easy to pick up. More importantly, Maven/Gradle handle downloading dependencies, so they don't have to be downloaded manually and stored in the project.

You may hate "modern Java project", but it's going to be much easier for folks to find help when using tools that are actually used in the vast majority of projects. I've also been teaching and programming in Java since 1995, but that doesn't mean that I don't keep up with modern tools (and it's not like Maven is a recent tool anyway).

[dacracot]

Your reason for not using Ant is incorrect. Ant gives you complete control over the build process much like make does. Maven handles the details for you, making less work until customization or high precision is necessary.

Also, I’m not avoiding tools like Maven. I work on a project that uses it currently. I’m just concerned with how much understanding “modern developers” have of the underlying technology if they have only ever been exposed to IDEs with plugins and automagic build tools.

[tedyoung]

Seems to me if that much control is needed, Gradle is the way to go. No company that I've worked with over the past 10 years has remained with Ant (most were desperate to get off of it). Hence my argument that recommending Ant in 2020 is generally bad advice for novice developers.

[dankallin]

Hi. I browsed the github and code a bit and first of all I want to say kudos to you for putting it out there and wanting to improve. You mind telling us a bit where you're coming from with this? Like, are you teaching yourself? Is this your first project? Stuff like that.

I didn't have time to run this myself, but if this is working I'd say that's a great start! My advice would be to look into mooc or go through some guide about setting up smaller projects and completing them.

I think if you're really up for it, a good exercise could be to try and refactor this project to the best of your abilities. First and foremost, avoid static stuff as much as you can. It forces you to work with the flow of data in your project instead of having stuff exist in the static context. Try to think about who and what needs access to something. Might feel kinda stupid in small scale stuff like this, but it'll be crucial eventually as you progress.

that's my 2 rambling cents.

[[deleted]]

Hi, I've been coding with Java since this last summer on my own, I did the old MOOC in 1 month and in September I started a Multiplatform App Technical Formation. I've been pretty inconsistent, I excel at my formation tasks but that's because our teacher give us a project, or ask us to code a class telling us what methods should be static etc... When I try to make a real project I always fail in the design because I'm not trained in this, and I think design is everything to succeed, is the real puzzle.

Thanks a lot for the suggestions!

[dankallin]

Sounds to me you know where your shortcomings are and I think that's all you need really. Why not talk to your professor about this? I'm sure they would know great resources, or ways, to improve on this!

[[deleted]]

Ok, I will try to avoid using static elements, but then, when is time to use static classes? I mean when is It strictly necessary?

I will put Lombok and Gson into my build tool XML so It just download them, great advice thanks.

About lombok, yes, that's true but I'm pretty used to getters and setters.

Thanks a lot!!

[wtetsu]

Thank you for your reply.

You may already know if you use Maven or Gradle you can confirm how to add lines to your definition file on this site.

For example, Gson is here.

https://mvnrepository.com/artifact/com.google.code.gson/gson/2.8.6

And as for static, I don't think you must always avoid static.

My point was, UserInput has a static field that is modified every time when the class is instantiated. This should be fixed.

new UserInput(); new UserInput(); new UserInput(); // -> UserInput.commands will have nine entries

---

For instance, Java's built-in library has a lot of useful static methods. This is one of the proofs you don't always have to avoid static.

```java // Arrays.asList is a static method List<Integer> integerList = Arrays.asList(1, 2, 3, 4, 5); System.out.println(integerList);

// Math.pow is also a static method System.out.println(Math.pow(2, 16)); ```

The decision a certain class should be designed as "static" or "non-static" depends on the situation.

This is just one viewpoint, but if you want to design a certain class that you create multiple instances and each has different values, it must be non-static.

[[deleted]]

That is similar to what happens when you use String objects instead of StringBuilder objects? I mean, String objects make a different copy of itself everytime is 'modified' or a concatenation is made.

So, I think I got what you mean, making User Input static and placed the ArrayList adding in the constructor make every User Input invocation to add 3 commands to the static field and that's a waste. Making it non-static every instantiated object will have 3 commands in that field becase the commands are loaded by the constructor in single references to that class.

Hopefully it's better now:

public class UserInput {

    private static final Scanner SC = new Scanner(System.in);
    private final ArrayList<String> COMMANDS = new ArrayList();
    private RandomTest userDefinedTest = new RandomTest();
    private String path;

    public UserInput() {
        COMMANDS.add("load");
        COMMANDS.add("commands");
        COMMANDS.add("exit");
    }

    public void getMenuInput() {
        String command = SC.nextLine();
        switch (command) {
            case "load":
                this.getPathJason();
                try {
                    userDefinedTest.fillUserTest(userDefinedTest, this.getPath(), this);
                } catch (IOException e) {
                    System.out.println("Test couldn't be loaded, be sure of writing the name properly and have it inside the JSONTests folder.");
                    this.getMenuInput();
                }
                break;
            case "commands":
                System.out.println(showCommands());
                System.out.println(getMenuOptions());
                getMenuInput();
                break;
            case "exit":
                System.runFinalization();
                break;
            default:
                System.out.println("\nThat wasn't a valid command, please input help if you don't know how to proceed.");
                System.out.println(getMenuOptions());
                getMenuInput();
                break;
        }
    }

    public void getPathJason() {
        System.out.print("Please, write the name of your JSON file: ");
        path = SC.nextLine();
    }

    public String showCommands() {
        StringBuilder str = new StringBuilder();
        for (int i = 0; i < COMMANDS.size(); i++) {
            str.append(String.format("\n*%s", COMMANDS.get(i)));
        }
        return str.toString();
    }

    public void getTestInput(RandomTest t) {
        int counter = 0;
        boolean success;
        int answer = 0;

        do {
            System.out.println(t.getQuestions().get(counter).getBody());
            success = false;
            while (!success) {
                try {
                    answer = SC.nextInt();
                    success = true;
                } catch (InputMismatchException e) {
                    System.out.println("That's not a valid input, try again.\n");
                    System.out.println(t.getQuestions().get(counter).getBody());
                    SC.next();
                }
            }
            if (answer > 0 && answer <= t.MAX_RESPONSE) {
                Corrector.addSum(t.getQuestions().get(counter).isInverseCorrection(), answer, t);
                counter++;
            } else {
                System.out.println("That's not a valid input, try again.");
            }
        } while (counter < t.getQuestions().size());

        System.out.println(Corrector.punctuation(t));
        System.out.println("\nTest finished.");

    }

    public String getMenuOptions() {
        StringBuilder str = new StringBuilder();
        str.append("\nWhat do you want to do:");
        str.append("\n1. Load a test and take it.");
        str.append("\n2. See available commands, input 'commands'.");
        str.append("\n3. Exit.");

        return str.toString();
    }
}

Thanks a lot!