Why does my Password generation code not work as expected?

Question

I want to generate a unique password each time. I am using this code for generating the password.

import java.util.Random;
public class PasswordGenerator
{
    public static String generatePassword()
    {
        Random r[] = new Random[8];
        r[0] = new Random(1234567);
        r[1] = new Random(7654321);
        r[2] = new Random(-1234567);
        r[3] = new Random(-7654321);
        r[4] = new Random(5463721);
        r[5] = new Random(2743615);
        r[6] = new Random(-9753214);
        r[7] = new Random(-3125769);
        Random x = new Random(2325671);
        StringBuilder password = new StringBuilder();
        int length = x.nextInt(5)+9;
        password.setLength(length);
        for(int i=0;i<length;i++)
        {
            x.setSeed(r[i%8].nextInt(500)*r[4].nextInt(900));
            password.setCharAt(i,(char)(r[x.nextInt(256)%8].nextInt(95)+32));
        }
        return password.toString();
    }
}

Code where the generatePassword() is called (if it is important)

public void actionPerformed(ActionEvent event)
    {
        if(event.getSource() == generate)
        {
            String userName = username.getText();
            if(userName.isEmpty() || username == null)
            {
                JOptionPane.showMessageDialog(null,"username not entered\nFirst enter your username","ERROR",JOptionPane.ERROR_MESSAGE);
                username.requestFocus();
                username.selectAll();
                return;
            }
            else if(userName.length() <=5)
            {
                JOptionPane.showMessageDialog(null,"Bad Username.\nUsername should be atleast six characters long.","ERROR",JOptionPane.ERROR_MESSAGE);
                username.requestFocus();
                username.selectAll();
                return;
            }
            else
            {
                String passwd = PasswordGenerator.generatePassword();
                password.setText(passwd);
                return;
            }
        }
        else if(event.getSource() == submit)
        {
            String passwordField = password.textField();
            if(passwordField.isEmpty() || passwordField == null)
            {
                JOptionPane.showMessageDialog(null,"Please Generate your password first by clicking on the \"Generate\" button.",JOptionPane.ERROR_MESSAGE);
                generate.requestFocus();
                return;
            }
            else
            {
                //do something...
            }
        }
    }

Each time it generates the same password, even when I recompile it. What should I actually modify to generate a unique password each time ?

Finally working code...

import java.util.Random;
public class PasswordGenerator
{
    public static String generatePassword()
    {
        Random r[] = new Random[8];
        for(int i=0;i<8;i++)
            r[i] = new Random();
        Random x = new Random();
        StringBuilder password = new StringBuilder();
        int length = x.nextInt(5)+9;
        password.setLength(length);
        for(int i=0;i<length;i++)
        {
            x.setSeed(r[i%8].nextInt(500)*r[4].nextInt(900));
            password.setCharAt(i,(char)(r[x.nextInt(256)%8].nextInt(95)+32));
        }
        return password.toString();
    }
}

A special Thanks to @reimeus and @Jon Skeet

Answer 1

Each time it generates the same password even when i recompile it. What should i actually modify to generate a unique password each time ?

You're explicitly providing the same seed to each of your 9 instances of Random:

Random r[] = new Random[8];
r[0] = new Random(1234567);
r[1] = new Random(7654321);
r[2] = new Random(-1234567);
r[3] = new Random(-7654321);
r[4] = new Random(5463721);
r[5] = new Random(2743615);
r[6] = new Random(-9753214);
r[7] = new Random(-3125769);
Random x = new Random(2325671);

It's not clear why you've even got more than one instance of Random, but you shouldn't be specifying the same seed - that will guarantee that you get the same results every time. Just use the Random constructor which doesn't take a seed, and that will pick a seed based on the current time (with some jiggery-pokery in modern versions to avoid using the same seed if you call the constructor multiple times in quick succession.)

It looks like you're doing all kinds of "clever" messing around to try to make the data more random - setting one seed based on a the results of calling next on a different instance, etc. You've made the code harder to understand, but no more random. You're still using predetermined seeds, with a deterministic RNG. There's no source of variation there.

Additionally, for sensitive information, you should use SecureRandom instead of Random.

Answer 2

This is because you instantiated your Random instanced with the fixed seed value:

r[0] = new Random(1234567); ...

From Random JavaDoc:

If two instances of Random are created with the same seed, and the same sequence of method calls is made for each, they will generate and return identical sequences of numbers.