Checking number of words in a string and assigning them to a value

Question

I am learning Java and trying to write a program to check the number of words in a string and am trying append them to create username and email id. If the number of words in the name are 3 then the first name is the 1st word and last name is the 3rd word, where as when the number of works in the name are 2 the First name is the 1st word and the Last name is the 2nd word. To differentiate this I've used the IF condition. I just wanted to find out if the code is good enough or if there's a better way of doing it. Code is as below :

import java.io.*;
public class CreateUsername {
 public static void main(String[] args) throws IOException{
 BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
 System.out.println("Provide Name");
 String Username = br.readLine();
 char[] arr = Username.toCharArray();
 int i = 0;
 boolean notCounted = true;
 int counter = 0;
 while (i < arr.length) 
 {
 if (arr[i] != ' ') 
 {
 if (notCounted) 
 {
 notCounted = false;
 counter++;
 }
 } 
 else 
 {
 notCounted = true;
 }
 i++;
 }
 System.out.println("words in the string are : " + counter);
 //Split the names and assign the Windows Username
 String FirstName = "";
 String firstLetterSurname = "";
 String Surname = "";
 String WindowsUsername = "";
 String EmailID = "";
 if(counter == 2)
 {
 System.out.println("Inside for count 2");
 FirstName = Username.split(" ")[0];
 Surname = Username.split(" ")[1];
 firstLetterSurname = String.valueOf(Surname.charAt(0));
 WindowsUsername = FirstName + "" + firstLetterSurname.toUpperCase();
 EmailID = FirstName + "." + Surname + "@test.com";
 }
 if(counter == 3)
 {
 FirstName = Username.split(" ")[0];
 Surname = Username.split(" ")[2];
 WindowsUsername = FirstName + "" + firstLetterSurname.toUpperCase() ;
 EmailID = FirstName + "." + Surname + "@test.com";
 }
 System.out.println("Windows Username is : " + WindowsUsername);
 System.out.println("Email ID is : " + EmailID);
 }
}

Answer 1

The code is pretty clean, if you started coding very recently, you can be quite happy with it.

But please note that some of your variables names start with an uppercase letter, as a reminder java use SCREAMING_SNAKE_CASE for static final variables, CamelCase for class name (or object type if you prefer), and camelCase (aka lowerCamelCase) for local variables, fields and method name.

Also if the user input only one word then your code fails (which is not necessarily bad nor wrong)... silently (which is bad).

You should print a warning message in such case.

---

Step by step review

public class CreateUserName {

CreateUserName sounds like a complete sentence which you should avoid for a class name. Consider renaming your class to UserNameCreator, UserNameParser, etc...

BufferedReader br = new BufferedReader(new InputStreamReader(System.in));

That's quite complicated just to read something from the console consider using Scanner for such a simple use case

char[] arr = Username.toCharArray();
int i = 0;
boolean notCounted = true;
int counter = 0;
while (i < arr.length) 
{
 if (arr[i] != ' ') 
 {
 if (notCounted) 
 {
 notCounted = false;
 counter++;
 }
 } 
 else 
 {
 notCounted = true;
 }
 i++;
}

First and foremost, you should put that into its own function. Secondly, good job in considering the case where multiple space are following one another :) sadly, if you indeed have multiple space it'll fail later in your code :( maybe you shouldn't only consider space but also tab ? (char for tab is '\t')

Thirdly, you didn't have to use an array, there is the method charAt inside String.

String FirstName = "";
String firstLetterSurname = "";
String Surname = "";
String WindowsUsername = "";
String EmailID = "";

Copying this part just to remind you to use camelCase ;)

if(counter == 2)
{
 //...

Use if / else if and consider adding a else to print a warning message if the input of the user is invalid. Never trust user inputs ;)

As said earlier, if the user input multiple space in a rows, this part may fail as you'll use charAt(0) on the empty string.

Also, firstLetterSurname is never written to if counter is 3 which is likely a bug.

Lastly, you shouldn't split the variable twice. Use split once and store the result in a temporary variable like this :

String[] splitted = userName.split(" ");
FirstName = splitted[0];
Surname = splitted[1];

---

Special note

Remember that people's names aren't as easy to manipulate as they seem : some people have space in their first name, others have multiple "middle name" (in France for example, we can have as many surname as wanted, though we use only one).

Also some family name may not be case sensitive (Victor D'Hondt is sometime written Victor d'Hondt or even Victor D'hondt). Something to consider if you ever want to add name comparison.

• SR1092: Thank you so much.. There is definitely a lot that I've learnt from this. Truly appreciate the detailed description.
• Ronan Dhellemmes: @SR1092 no problem :) consider posting a follow-up question once you have modified your code so we can give you more advices

Answer 2

That is alrdeay a good job. Just a few "blocking" points :

Blocking: Apply the conventions and avoid one big public static void main(String[] args). Fields are lowerCamelCase and static methods that returns nothing are hard to test.

Simplification: You don't need the counting logic, String.split accept a regex, you can use \s+ to split on one or many whitespace (tab and insecable included).

The two if ( counter==2 ) and if ( counter==3 ) can be replaced with on if .. else if .., or a switch because you will never enter the two blocks. When looking at this part, who is the core of your program, it seems that there is a hughe difference between the generation from two or three words. In fact, there is only tow subtle differences. firstName is always the same, an surname is just tjhe last word words[words.length-1]. Make thay clearer by assigning them before your two if.

Design From an OO perspective (and testability), think about your needs. You want something that receive a set of words and return a username and email. This class will basically extract all "words" form the names via String.split and generate one user name and one email when there is two or three words.

Returning two values is not easy in Java, you can return an array but it is not elegant. You may introduce a User class with those two fields.

Separating the username and email generation is also a good idea because it will be more easier to spot them if needed to change. You can still guard them based on the number of words.

class CreateUsername {
 User create(String names) {
 String[] words = names.split("\\s+");
 if ( words.length<2 || words.length>3 ) 
 return null; // Or throw an exception
 return new User(getUserName(words), getEmail(words));
 }
 private String getUserName(String[] words) {
 // It seems that when there is three words the login is also the fisrt name..
 String firstLetterSurname = words.length==2
 ?Character.toString(words[1].charAt(0)):"";
 return String.format("%1$s%2$s", words[0], firstLetterSurname);
 }
}

You can easily test this class and use it from your main method. And maybe rename it to UserGenerator for consitency.

• SR1092: Thank you so much.. There is definitely a lot that I've learnt from this. Truly appreciate the detailed description.
• gervais.b: I made "my" version if you want. github.com/gervaisb/stackexchange-codereview/blob/165736/src/…