TransWikia.com

Pin and Password Generator JAVA

Code Review Asked by Brian Smithers on October 27, 2021

I am reaching my one year on programming and decided to create a simple pin and password program to better help me understand arrays. As far as coding standards, best practices, and functionality what else could I do to this to make it better?


public static void main(String []args){

    choicePrompt(); // Prompt to choose pin or password.
    menuMethod(); // Method to catch user input if not (1) PIN, (2) Password or (3) Exit.

}

static void choicePrompt() {
    System.out.printf("Password Generator:%n");
    System.out.printf("1 - Create PIN%n");
    System.out.printf("2 - Create Password%n");
    System.out.printf("3 - EXIT%n");
}

// Method accepts (1), (2), (3) or handles any input exception.
static void menuMethod() {
    int input = 0;  // Initialize integer input value.
    while (input != 1 && input != 2 && input != 3) { // User must choose these options to progress.
        Scanner obj = new Scanner(System.in); // Creates object for user input.
        try { // Try-catch block to catch incorrect input value...i.e. not an integer.
            input = obj.nextInt(); // 1 - PIN // 2 - Password // 3 - EXIT.
            if (input == 1 || input == 2 || input == 3) {
                inputMethod(input); // Successful input
            }
            else {
                invalidValuePrompt(); // Remind user to select correct value.
                menuMethod(); // Recursive call gives user option to create PIN, Password or EXIT.
            }
        } catch (InputMismatchException e) { // If input is not an integer.
            // Skip to finally block
        } finally {
            invalidValuePrompt(); // Remind user to select correct value.
            menuMethod(); // Recursive call gives user option to create PIN, Password or EXIT.
        }
    }
}

// Method for menuMethod()
static void invalidValuePrompt() {
    System.out.printf("Please enter: %n1 - Create PIN%n2 - Create Password%n3 - EXIT%n");
}

// Method takes users input and starts pinGenerator(), passwordGenerator() or EXITS program.
static void inputMethod(int input) { // If statement selects PIN, Password or EXIT option according to input.
    Scanner obj = new Scanner(System.in); // Creates object for user input.
    if (input == 1) { // PIN generator
        int pinLength = 0;
        System.out.printf("Enter pin length: (4 - 32)%n");
        try {
            pinLength = obj.nextInt();
            while (pinLength < 4 || pinLength > 32) {
                System.out.printf("Please enter: (4-32)%n");
                pinLength = obj.nextInt();
            }
        }  catch (InputMismatchException e) { // If input is not an integer.
            // Skip to finally block
        } finally { // Executes if user does not enter valid password length.
            if (pinLength < 4 || pinLength > 32) {
                invalidValuePrompt();
                inputMethod(input); // Recursive call to method.
            }
        }
        pinGenerator(pinLength); // PIN generates and is printed.
        System.exit(0); // Program is finished.
    }
    else if (input == 2) { // Password generator
        int passwordLength = 0;
        System.out.printf("Enter password length: (6-64)%n");
        try {
            passwordLength = obj.nextInt();
            while (passwordLength < 4 || passwordLength > 64) {
                invalidValuePrompt();
                System.out.printf("Please enter: (4-64)%n");
                passwordLength = obj.nextInt();
            }
        } catch (InputMismatchException e) {
            // Skip to finally block
        } finally {
            if (passwordLength < 4 || passwordLength > 64) {
                invalidValuePrompt();
                inputMethod(input); // Recursive call to method.
            }
        }
        passwordGenerator(passwordLength); // Password generates and is printed.
        System.exit(0); // Program exits.
    }
    else if (input == 3) { // EXIT program.
        System.exit(0);
    }
}

// Method used during try-catch blocks when user inputs a non-integer value.
static void invalidValue() {
    System.out.printf("Invalid value.%n");
}

// Method generates pin from an array. passwordLength is determined in previous
// method by user.
static void pinGenerator(int passwordLength) {
    SecureRandom random = new SecureRandom();
    int[] myArray2 = {1, 2, 3, 4, 5, 6, 7, 8, 9, 0};

    // Array prints random index with SecureRandom Class.
    for (int i = 0; i < passwordLength; i++) {
        int randomNum = random.nextInt(myArray2.length);
        System.out.print(myArray2[randomNum]);
    }
}

// Method generates a random password using SecureRandom class by utilizing four arrays with alphabetical
// characters, numerical values and special characters.
static void passwordGenerator(int passwordLength) {

    SecureRandom random = new SecureRandom();

    String[] myArray = {"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n","o",
            "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z"};
    int[] myArray2 = {1, 2, 3, 4, 5, 6, 7, 8, 9, 0};
    String[] myArray3 = {"A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O",
            "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"};
    String[] myArray4 = {"!", "@", "#", "%", "&", "=", "?", "-"};

    // For loop generates password from array using switch class.
    for (int i = 0; i < passwordLength; i++) { // passwordLength equals users input.
        int valueChoice = 1 + random.nextInt(4); // Bounds account for all arrays in switch case.
        switch(valueChoice) {
            case 1: // Random lowercase letter is chosen from array.
                int randomLowerCaseLetter = random.nextInt(myArray.length);
                System.out.print(myArray[randomLowerCaseLetter]);
                break; // Restarts loop and generates next value.
            case 2: // Random numerical value (0-9) is chosen from array.
                int randomNum = random.nextInt(myArray2.length);
                System.out.print(myArray2[randomNum]);
                break;
            case 3: // Random uppercase letter is chosen from array.
                int randomUpperCaseLetter = random.nextInt(myArray3.length);
                System.out.print(myArray3[randomUpperCaseLetter]);
                break;
            case 4: // Random special character is chosen from array.
                int randomSymbols = random.nextInt(myArray4.length);
                System.out.print(myArray4[randomSymbols]);
                break;
        }
    }
}

2 Answers

Overall good implementation, well commented and easy to understand.

These are my suggestions:

  1. You can model the user choice as Enum (more readable)
  2. The code to request an integer from the user is duplicated many times, better to use a single method and reuse it
  3. InvalidValue() method is never used
  4. Better to use System.out.println instead of System.out.printf (no need of %n)
  5. No need to call System.exit(0) the program ends automatically
  6. For the methods passwordGenerator and pinGenerator is better to return a String and print it rather than use System.out.print inside (easier to reuse in the future)
  7. myArray2 in pinGenerator method? Maybe a typo
  8. As @tinstaafl suggested you can generate numbers and letters without using arrays (more readable)

This is the code refactored:

public static void main(String[] args) {
    choicePrompt(); // Prompt to choose pin or password.
    menuMethod(); // Method to catch user input if not (1) PIN, (2) Password or (3) Exit.
}

static void choicePrompt() {
    System.out.println("Password Generator:");
    System.out.println("1 - Create PIN");
    System.out.println("2 - Create Password");
    System.out.println("3 - EXIT");
}

// Method accepts (1), (2), (3)
static void menuMethod() {
    int input = requestIntBetween(1, 3, invalidValuePrompt());
    inputMethod(input);
}

static int requestIntBetween(int start, int end, String errorMessage) {
    int input = Integer.MIN_VALUE;
    Scanner obj = new Scanner(System.in);
    while (input < start || input > end) {
        if (obj.hasNextInt()) {
            input = obj.nextInt();
            if (input < start || input > end) {
                System.out.println(errorMessage);
            }
        } else {
            obj.nextLine();
            System.out.println(errorMessage);
        }
    }

    return input;
}

// Method for menuMethod()
static String invalidValuePrompt() {
    return "Please enter: n1 - Create PINn2 - Create Passwordn3 - EXIT";
}

// Method takes users input and starts pinGenerator(), passwordGenerator() or exits
static void inputMethod(int input) { 
    switch (Choice.from(input)) {
    case CREATE_PIN:
        System.out.println("Enter pin length: (4 - 32)");
        int pinLength = requestIntBetween(4, 32, "Please enter: (4-32)");
        String pin = pinGenerator(pinLength); // PIN generates and is printed.
        System.out.println(pin);
        break;
    case CREATE_PW:
        System.out.println("Enter password length: (6-64)");
        int passwordLength = requestIntBetween(6, 64, "Please enter: (6-64)");
        String pw = passwordGenerator(passwordLength); // Password generates and is printed.
        System.out.println(pw);
        break;
    case EXIT:
        // do nothing and exit
        break;
    }
}

// Method generates pin randomly. passwordLength is determined in previous
// method by user.
static String pinGenerator(int pinLength) {
    SecureRandom random = new SecureRandom();
    StringBuilder sb = new StringBuilder();
    for (int i = 0; i < pinLength; i++) {
        sb.append(random.nextInt(10));
    }
    return sb.toString();
}

// Method generates a random password using SecureRandom class by utilizing
// characters, numerical values and special characters.
static String passwordGenerator(int passwordLength) {
    SecureRandom random = new SecureRandom();
    String letters = "abcdefghijklmnopqrstuvwxyz";
    String[] specialCharacters = { "!", "@", "#", "%", "&", "=", "?", "-" };
    StringBuilder sb = new StringBuilder();
    // For loop generates password from array using switch class.
    for (int i = 0; i < passwordLength; i++) { // passwordLength equals users input.
        int valueChoice = 1 + random.nextInt(4); // Bounds account for all arrays in switch case.
        switch (valueChoice) {
        case 1: // Random lowercase letter
            sb.append(letters.charAt(random.nextInt(letters.length())));
            break; // Restarts loop and generates next value.
        case 2: // Random numerical value (0-9)
            sb.append(random.nextInt(10));
            break;
        case 3: // Random uppercase letter
            sb.append(letters.toUpperCase().charAt(random.nextInt(letters.length())));
            break;
        case 4: // Random special character is chosen from array.
            int randomSymbols = random.nextInt(specialCharacters.length);
            sb.append(specialCharacters[randomSymbols]);
            break;
        }
    }
    return sb.toString();
}

And the class Choice:

public enum Choice {
    CREATE_PIN(1), CREATE_PW(2), EXIT(3);

    private int value;

    Choice(int value) {
        this.value = value;
    }

    public int getValue() {
        return this.value;
    }

    public static Choice from(int choice) {
        return Stream.of(Choice.values())
                  .filter(c -> c.value == choice)
                  .findFirst().orElse(null);
    }
}

Answered by Marc on October 27, 2021

A few things I noticed:

You're printing the password directly to the console. I would suggest, you get much more versatility by returning the password as a string. This way the user of the function can decide how or if the password gets displayed. For instance, displaying the password on a web page, or hashing it and storing it a database.

You haven't set any format limits on the password, just the length. Typically, besides the length, there are requirements for a minimum number of each type of character, capitals, lowercase, digits, and punctuation.

You wastefully repeat the array holding the digits for the pinGenerator function. In fact you can dispense with all the arrays except the one for punctuation. Simply set the max limit of random.nextInt to 26 or 10 and add 'a','A','0' to get the appropriate char.

I would suggest holding all the different types of characters in char[]'s. This way you can create a char[] for the password and create a string from that.

Likewise with your menu choices, expecting a char requires a lot less validating.

Instead of using a try-catch block and all those if-else blocks to validate the users choice(s), a simple switch will do the job much more easily. The try-catch block isn't made for this kind of validation and adds an extra degree of overhead to your code that isn't needed.

Only use 1 instance of the SecureRandom class as a private global field. I'ts wasteful to keep seeding a new instance every time the functions are run.

Answered by user33306 on October 27, 2021

Add your own answers!

Ask a Question

Get help from others!

© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP