2
\$\begingroup\$

I made a very simple user account registration program that works in the terminal using no external libraries other than java.util.Scanner. I want to know what improvements I can make to this code to make it more efficient and more readable:

private static void newAccount(){
    boolean next = false;
    System.out.println("Welcome to the User Account registration.");
    Scanner scan = new Scanner(System.in);
    String username = null;
    String password = null;
    String email = null;
    while(!next){
        System.out.println("Username:");
        username = scan.nextLine();
        if(username.length() < 3 || username.length() > 20){
            System.out.println("Your username must be between 3-20 characters in length");
        } else {
            next = true;
        }
    }
    next = false;
    while(!next){
        System.out.println("Password:");
        password = scan.nextLine();
        if(password.length() < 8){
            System.out.println("Your password must be at least 8 characters in length");
        } else {
            next = true;
        }
    }
    next = false;
    while(!next){
        System.out.println("Email:");
        email = scan.nextLine();
        if(!email.contains("@")){
            System.out.println("Your email is invalid");
        } else {
            next = true;
        }
    }
    next = false;
    while(!next){
        System.out.println("Confirm Email:");
        String confirmEmail = scan.nextLine();
        if(!confirmEmail.equals(email)){
            System.out.println("This email does not match the email above");
        } else {
            next = true;
        }
    }
    next = false;
    while(!next){
        System.out.println("Does the following information look the same? (Y or N)\nUsername: " + username + "\nPassword: " + password + "\nEmail: " + email);
        String response = scan.nextLine();
        if(response.equals("Y") || response.equals("y")){
            System.out.println("Congratulations on completing the User Account Registration!");
            next = true;
        } else if(response.equals("N") || response.equals("n")){
            newAccount();
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$

The logic to request the user data is fairly similar. It can be extracted to a method (in pseudocode):

inputInvalid = true
while (inputInvalid) {
  print(inputToBeRequested)
  input = requestInput()
  if (!isValid(input))
    print(errorMessage)
  else
    inputInvalid = false 
}

Now, you just need to fill in the gaps. For this, you could use a consumer/supplier approach, or the one I'd rather go with, as I think it makes the code more readable, the method template pattern:

import java.util.Scanner;

public class UserRegistration {
    
    private String username;
    private String password;
    private String email;
    
    public static void main(String[] args) {
        new UserRegistration().newAccount();
    }
    
    public void newAccount() {
        UsernameRetriever usernameRetriever = new UsernameRetriever();
        PasswordRetriever passwordRetriever = new PasswordRetriever();
        EmailRetriever emailRetriever = new EmailRetriever();
        ConfirmationRetriever confirmationRetriever;
        
        do {
            username = usernameRetriever.requestData();
            password = passwordRetriever.requestData();
            email = emailRetriever.requestData();
            confirmationRetriever = new ConfirmationRetriever(username, password, email);
        } while (!confirmationRetriever.isResponseYes(confirmationRetriever.requestData()));
        
        System.out.println("Registered user with: "
                + "Username=" + username + ", "
                +" Password=" + password + ", "
                + "Email=" + email);
    }
    
    static abstract class UserDataRetriever {
        
        private static Scanner scanner = new Scanner(System.in);
        
        public String requestData() {
            String input;
            boolean inputIsInvalid;
            do {
                System.out.println(getDataRequestMessage());
                input = scanner.nextLine();
                inputIsInvalid = !isValid(input);
                if (inputIsInvalid) {
                    System.out.println(getInvalidInputMessage());
                }
            } while (inputIsInvalid);
            
            return input;
        }

        protected abstract String getInvalidInputMessage();

        protected abstract String getDataRequestMessage();

        protected abstract boolean isValid(String input);
    }
    
    static class UsernameRetriever extends UserDataRetriever {
        
        @Override
        protected String getInvalidInputMessage() {
            return "Your username must be between 3-20 characters in length";
        }
        
        @Override
        protected String getDataRequestMessage() {
            return "Username:";
        }
        
        @Override
        protected boolean isValid(String username) {
            return username.length() >= 3 && username.length() <= 20;
        }
    }
    
    static class EmailRetriever extends UserDataRetriever {
        
        private boolean confirmEmail = false;
        private String email;
        
        @Override
        public String requestData() {
            email = super.requestData();
            confirmEmail = true;
            super.requestData();
            return email;
        }
        
        
        @Override
        protected String getInvalidInputMessage() {
            return confirmEmail ? "This email does not match the email above" : "Your email is invalid" ;
        }

        @Override
        protected String getDataRequestMessage() {
            return confirmEmail ? "Confirm Email:" : "Email:";
        }

        @Override
        protected boolean isValid(String email) {
            return confirmEmail ? this.email.equals(email) : email.contains("@");
        }
    }
    
    static class PasswordRetriever extends UserDataRetriever {

        @Override
        protected String getInvalidInputMessage() {
            return "Your password must be at least 8 characters in length";
        }

        @Override
        protected String getDataRequestMessage() {
            return "Password:";
        }

        @Override
        protected boolean isValid(String password) {
            return password.length() >= 8;
        }
    }
    
    static class ConfirmationRetriever extends UserDataRetriever {
        
        private String username;
        private String password;
        private String email;

        public ConfirmationRetriever(String username, String password, String email) {
            this.username = username;
            this.password = password;
            this.email = email;
        }

        @Override
        protected String getInvalidInputMessage() {
            return "Please, answer with just 'Y' or 'N'";
        }

        @Override
        protected String getDataRequestMessage() {
            return "Does the following information look the same? (Y or N)\n"
                    + "Username: " + username + "\n"
                    + "Password: " + password + "\n"
                    + "Email: " + email;
        }

        @Override
        protected boolean isValid(String response) {
            return isResponseYes(response) || isResponseNo(response);
        }
        
        public boolean isResponseYes(String response) {
            return response.equals("Y") || response.equals("y");
        }
        
        public boolean isResponseNo(String response) {
            return response.equals("N") || response.equals("n");
        }
    }
}

Now, I would personally have each class on their own file. Rather than them being inner classes in UserRegistration. But I wrote it like that so that by simply copy-pasting, the code can be run.

Also, we could argue that the code is now way longer. However, that is not necessarily worse. The way the code is written now, if you just read the newAccount method, it is fairly easy to tell what is going on. The UserDataRetriever has a very simple and specific purpose, and each of its children are really easy to debug. Say, for example, the email validation needs to be changed now. Then, you just need to look for the isValid(String) method in EmailRetriever, rather than having to read through the newAccount method and look for where the email is validated.

\$\endgroup\$

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.