4
\$\begingroup\$

I wrote this program recently to generate random passwords for myself. It works well but it's been a long time since I wrote any java and am just looking on ways to clean up the code. I'm also not sure how truly "random" the password generation is or if it is too excessive.

import java.util.Random;
import java.util.Scanner;

final class RNG
{
    final public static void main(String[] args)
    {
        System.out.print("This program generates between 1 and 100 RANDOM and SECURE passwords.\nPasswords are not stored in any way.\nWRITE DOWN YOUR NEW PASSWORDS!\nPassword length must be between 12 and 120.\n\n");
        System.out.print("Please enter the length for your new password(s): ");
        Scanner input = new Scanner(System.in);
        if(input.hasNextLine())
        {
            System.out.print("Please enter the number of passwords you wish to create: ");
            Scanner number = new Scanner(System.in);
            RNG myPW = new RNG();
            myPW.testLength(input, number);
        }
    }
    final private void testLength(Scanner test, Scanner amount)
    {
        if(test.hasNextInt() && amount.hasNextInt())
        {
            int PWlength = test.nextInt();
            int numberOfPW = amount.nextInt();
            if ((PWlength >= 12 && PWlength <= 120) && numberOfPW > 0 && numberOfPW <= 100)
            {
                RNG myPW = new RNG();
                myPW.PWGenerator(PWlength, numberOfPW);
            }
            if ((PWlength < 12 || PWlength > 120) && numberOfPW > 0 && numberOfPW <= 100)
            {
                System.out.println("ERROR! LENGTH OF PASSWORD MUST BE BETWEEN 12 AND 120\n");
                restart();
            }
            if ((PWlength >= 12 && PWlength <= 120) && numberOfPW <= 0)
            {
                System.out.println("ERROR! NUMBER OF PASSWORDS YOU WISH TO GENERATE MUST BE GREATER THAN ZERO\n");
                restart();
            }
            if ((PWlength >= 12 && PWlength <= 120) && numberOfPW > 100)
            {
                System.out.println("ERROR! NUMBER OF PASSWORDS YOU WISH TO GENERATE MUST BE LESS THAN OR EQUAL TO 100\n");
                restart();
            }
            if((PWlength < 12 || PWlength > 120) && numberOfPW <= 0)
            {
                System.out.println("ERROR! LENGTH OF PASSWORD MUST BE BETWEEN 10 AND 120");
                System.out.println("ERROR! NUMBER OF PASSWORDS YOU WISH TO GENERATE MUST BE GREATER THAN ZERO\n");
                restart();
            }
            if((PWlength < 12 || PWlength > 120) && numberOfPW > 100)
            {
                System.out.println("ERROR! LENGTH OF PASSWORD MUST BE BETWEEN 10 AND 120");
                System.out.println("ERROR! NUMBER OF PASSWORDS YOU WISH TO GENERATE MUST BE LESS THAN OR EQUAL TO 100\n");
                restart();
            }
        }
        if(!test.hasNextInt() && amount.hasNextInt())
        {
            System.out.println("ERROR! LENGTH OF PASSWORD IS NOT AN INTEGER. PLEASE TRY AGAIN.\n");
            restart();
        }
        if(test.hasNextInt() && !amount.hasNextInt())
        {
            System.out.println("ERROR! NUMBER OF PASSWORDS IS NOT AN INTEGER. PLEASE TRY AGAIN.\n");
            restart();
        }
        else
        {
            System.out.println("ERROR! LENGTH OF PASSWORD IS NOT AN INTEGER. PLEASE TRY AGAIN.");
            System.out.println("ERROR! NUMBER OF PASSWORDS IS NOT AN INTEGER. PLEASE TRY AGAIN.\n");
            restart();
        }
    }
    final private void restart()
    {
        System.out.print("Please enter the length for your new password(s): ");
        Scanner input = new Scanner(System.in);
        if(input.hasNextLine())
        {
            System.out.print("Please enter the number of passwords you wish to create: ");
            Scanner number = new Scanner(System.in);
            RNG myPW = new RNG();
            myPW.testLength(input, number);
        }
    }
    final private void PWGenerator(int a, int b)
    {
        Random randomInt = new Random();
        Random randomLetterU = new Random();
        Random randomLetterL = new Random();
        Random randomChar = new Random();

        String[] alphabetUPPER = {"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[] alphabetlower = {"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[] characters = {"!", "@", "#", "$", "%", "^", "&", "*", "(", ")", "-", "_", "+", "=", "?", "/", "<", ">"};

        int selection;
        int c;
        int e = 1;

        System.out.println();

        while(e <= b)
        {
            System.out.print("Password #" + e + ": ");
            int d = 1;
            while(d <= a)
            {
                c = randomInt.nextInt(4);
                if (c == 0)
                {
                    selection = randomLetterU.nextInt(26);
                    System.out.print(alphabetUPPER[selection]);
                    d++;
                }
                if (c == 1)
                {
                    selection = randomLetterL.nextInt(26);
                    System.out.print(alphabetlower[selection]);
                    d++;
                }
                if (c == 2)
                {
                    System.out.print(randomInt.nextInt(10));
                    d++;
                }
                if (c == 3)
                {
                    selection = randomChar.nextInt(18);
                    System.out.print(characters[selection]);
                    d++;
                }
            } 
            System.out.println("\n");
            e++;
        }
        if(e > b)
        {System.exit(0);}
    }
}
\$\endgroup\$
2
\$\begingroup\$

General

RNG is a poor name for a class that generates passwords, not random numbers. PasswordGenerator would be better.

In idiomatic Java, { are on the same line, not on a newline. else is on the same line as }.

In idiomatic Java, method and variable names use camelCase. Avoid acronyms - they tend to make the code harder to read.

Using final indicates to the reader that variables will not be reassigned.

Your whitespace is inconsistent. In idiomatic Java, there is always whitespace between a control flow keyword (if, while) and the (.

Resources that can be closed, such as Scanner, should always be closed, either in a try-with-resources or a try-finally. You also only need one scanner.

ALL CAPS messages are hard to read. Use proper capitalization.

restart

This should be refactored - there's no reason to prompt the user here and also in main(). It would be cleaner to have two methods, each asking for one value and repeating until it is valid.

testLength

There's no reason to test every possible permutation of cases separately. Test each case once, and include the relevant error message for that case if it fails.

PWGenerator

You only need one Random instance. It should be passed around, for testing purposes.

a, b, c, d, and e are utterly meaningless. Try to use variable names that make it easier to understand what your code is doing.

You can make password generation a little more compact if you keep a String[][], rather than a String[].

You don't need the last if check - you can't exit the while loop until you're done generating passwords. Calling System.exit(0) explicitly is overkill - let the flow of the application end normally.

for loops might be more appropriate than the while loops. They better limit the scope of the counter variables.


If you make all these changes, your code might look more like:

final class PasswordGenerator {

    public static final void main(String[] args) {
        System.out.print("This program generates between 1 and 100 RANDOM and SECURE passwords.\nPasswords are not stored in any way.\nWRITE DOWN YOUR NEW PASSWORDS!\nPassword length must be between 12 and 120.\n\n");
        try (final Scanner input = new Scanner(System.in)) {
            new PasswordGenerator().generatePasswords(passwordLength(input), numberOfPasswords(input));
        }
    }

    private static int scanInt(final Scanner scanner) {
        while (!scanner.hasNextInt()) {
            System.out.print("Error! '" + scanner.next() + "' is not a number. Please try again: ");
        }
        return scanner.nextInt();
    }

    private static int passwordLength(final Scanner scanner) {
        System.out.print("Please enter the length for your new password(s): ");
        int passwordLength = scanInt(scanner);

        while (passwordLength < 12 || passwordLength > 120) {
            System.out.println("Error! Length of password mut be between 12 and 120\n");
            System.out.print("Please enter the length for your new password(s): ");
            passwordLength = scanInt(scanner);
        }

        return passwordLength;
    }

    private static int numberOfPasswords(final Scanner scanner) {
        System.out.print("Please enter the number of passwords you wish to create: ");
        int numberOfPasswords = scanInt(scanner);

        while (numberOfPasswords < 1 || numberOfPasswords > 100) {
            System.out.println("Error! Number of passwords must be between 1 and 100\n");
            System.out.print("Please enter the number of passwords you wish to create: ");
            numberOfPasswords = scanInt(scanner);
        }

        return numberOfPasswords;
    }

    private final void generatePasswords(final int desiredPasswordLength, final int desiredNumberOfPasswords) {
        final Random random = new Random();

        final String[][] passwordCharacters = {
                {"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"},
                {"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"},
                {"0", "1", "2", "3", "4", "5", "6", "7", "8", "9" },
                {"!", "@", "#", "$", "%", "^", "&", "*", "(", ")", "-", "_", "+", "=", "?", "/", "<", ">"},
        };

        System.out.println();

        for (int passwordNumber = 1; passwordNumber <= desiredNumberOfPasswords; passwordNumber++) {
            System.out.print("Password #" + passwordNumber + ": ");
            for (int currentPasswordLength = 0; currentPasswordLength < desiredPasswordLength; currentPasswordLength++) {
                final int characterType = random.nextInt(passwordCharacters.length);
                System.out.print(passwordCharacters[characterType][random.nextInt(passwordCharacters[characterType].length)]);
            }
            System.out.println("\n");
        }
    }
}
\$\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.