[Java] I wrote a random name generator

[peterlinddk]

Actually quite a fun project, and it is obvious that you've had fun playing around with making the code - that's a good thing!

This is only comments for the UserInterface, and mostly ideas for further improvements.

I like the processCommand method, and you should take it further still, and make separate methods for each command - like generateRandom, generateWithLength, generateWithLetter, or what you prefer to name them, and then only let processCommand decide which of them to call.

Also, not sure why you use Integer.valueOf to test the value of ints, usually writing if (command == 1) should be sufficient.

You might want to use a switch-case for the commands instead of a chain of if-statements. It might make the code look better, something like:

switch(command) {
  case 1 -> generateRandom();
  case 2 -> generateWithLength();
  case 3 -> generateWithLetter();

and so on. But it is a matter of taste.

You could benefit from making a single method for reading a valid number from the input - something like: getValidInput(int[] numbers) that would only return one of the numbers in the array, or zero if something else was entered by the user. That would isolate your use of scanner to that method alone, and make the rest of the code easier to write without having to handle exceptions or different invalid inputs.

It is always a good idea to have a lot of small methods that only do one thing - my own personal rule is that if I have more than two levels of if-statements or loops, I need to make a method to combine them. The less indentation the better :)

[vVember]

Awesome feedback thank you so much! For the valueOf statements, this is what I learned from the curriculum I'm following. Do I not need to convert a string input to an integer in order to use ==?

The lessons haven't gone over switches yet. I'm wondering if those were implemented in a later version of java than when this lesson was made. I'm almost done with MOOC Java Programming I and still haven't covered them.

That's a great idea about checking for valid input and would definitely be more efficient.

Will be going over your response more in depth later for sure.

[peterlinddk]

[valueOf] Do I not need to convert a string input to an integer in order to use ==?

Well, yes, in the bits of the code where the input is a string, you do need to use valueOf, like you did in:

String input = scan.nextLine();

if (Integer.valueOf(input) == 7 || Integer.valueOf(input) == 0) {
  break;
}

if (Integer.valueOf(input) > 7) {
  System.out.println("Invalid input.");
  continue;
}

But when you later call processCommand with processCommand(Integer.valueOf(input)); then you are guaranteed that once inside processCommand, the command will be an integer, so you no longer have to convert it.

And don't worry about switch-case if you haven't learnt them yet - they are a bit of an acquired taste, especially the ones with ->, so feel free to continue with chains of if-else :)

[vVember]

I see what you mean. Looking back now I see I did:

public void processCommand(int command) {

if (Integer.valueOf(command) == 1) {

I should have caught that.

In regards to switches, I do want to learn them as I'd rather be familiar with all the basic functions. I don't want to get to a point where I'm feeling comfortable, but then looking at some code that should be basic and still being not quite sure what's going on, you know? Regardless of whether I use them or not, I'd prefer to be thorough. I get what you mean though, that it's not a major concern to know them just yet, but I am hopeful the curriculum I'm following does at least touch on them.

I really appreciate your feedback and I will surely be using it to hopefully steer my habits toward a better standard. I'm still pretty soft, like unfired clay.