Random password generator

I just wrote my first Java class, and I wanted to share it with you and maybe get some quick check from more experiences coders than I am.

I want to learn OOP, and Java is just the tool I thought was the best to start with (it's very similar to C in syntax, which I'm used to, and it's spreading more and more, day by day).

You can obviously comment on Java bad habits and ways to improve the class, but consider the class structure and the abstraction of the problem I wanted to solve, the separation between in data methods, etc.. as my main point of concern.

Here's the class. The aim is to create a random password generator.

```
import java.util.Random;

public final class PasswordGenerator {

// DATAS
// characters with which the password will be composed
private static final int charactersSize = 100;
private static char [] characters = new char [charactersSize];

// keep the counts of used characters
private static int charactersCount = 0;

// size of the password to generate
private int passwordSize;

// CONSTRUCTOR
public PasswordGenerator( int passwordSize ) {

// set the password size
this.passwordSize = passwordSize;

// set the characters that will be used to generate the password
initCharacters();
}

// METHODS
// fill the array of characters that will be used to generate the password
private static char [] initCharacters() {
int i = 0;

// add 0-9
for ( int j = 48; j < 58; ++i, ++j, ++charactersCount ) {
characters[i] = (char) j;
}

// add @ + a-z
for ( int j = 64; j < 91; ++i, ++j, ++charactersCount ) {
characters[i] = (char) j;
}

// add A-Z
for ( int j = 97; j < 123; ++i, ++j, ++charactersCount ) {
characters[i] = (char) j;
}

return characters;
}

// generate a random password
public char [] get() {

// initialize the random number generator
Random rnd = new Random(

Your code does have its share of C-isms, but we will be able to fix that.

But first, if you are learning Java in order to understand Object Oriented Programming, pick a better language. Java's OO primitives are classes, abstract classes, interfaces and single inheritance. Since Java was designed, the world has moved on, and trait-based OO (instead of interfaces) is all the rage. The Scala language isn't very C-like, but has a wonderful object system. Another interesting language for OO is Ruby with it's metaclasses.

I assume you already know about the difference of static and non-static fields and methods. Whatever is marked as static belongs to the class and is shared among all objects of that class. Your fillCharacters static method only operates on the class level, yet you call it in an object constructor. Instead of refilling the characters each time you make a new generator, initialize that array once.

A horrible C-ism is that you make your characters array fixed size, in the hope that it will be large enough. But you only fill it with 63 characters! This is the way bugs are born. Java has multiple collections (like LinkedLists or ArrayLists) that resize themselves if necessary. I would do something like this:

private static char[] characters = initCharacters();

private static char[] initCharacters() {
  final int initialCapacity = 63;
  // a vector is a variable-size array
  final List chars = new Vector(initialCapacity);

  // add digits 0–9
  for (char c = '0'; c <= '9'; c++) {
    chars.add(c);
  }

  // add uppercase A–Z and '@'
  for (char c = '@'; c <= 'Z'; c++) {
    chars.add(c);
  }

  // add lowercase a–z
  for (char c = 'a'; c <= 'z'; c++) {
    chars.add(c);
  }

  // Copy the chars over to a simple array, now that we know
  // the length. The .toArray method could have been used here,
  // but its usage is a pain.

  final char[] charArray = new char[chars.size()];

  for (int i = 0; i < chars.size(); i++) {
    charArray[i] = chars.get(i).charValue();
  }

  return charArray;
}

private static void showCharacters() {
  System.err.println("The following " + characters.length + " characters are available: " + new String(characters));
}

What was that characters.length? Yes, Java keeps book about the size of your arrays, so you don't have to! Keeping extra variables around for the size is error prone. Also note that I allocate various objects in the initCharacters method – the garbage collector will clean up after me.

Because our character array has no unused fields, we don't need the charactersSize and charactersCount variables any longer.

Something that C doesn't have is generics, i.e. parameterized polymorphism. The arguments in angled brackets are type names, which can only be classes. Java has primitive types like char and int. These have corresponding object types like Character and Integer.

Your get method is mostly allright. I'd call it nextPassword, and would rename rnd to random. The algorithm is fine.

One thing I'd do differently would be not using much object orientation for this problem. Creating a new PasswordGenerator for different sizes is silly. I'd rather use that functionality like

System.out.println(PasswordGenerator.nextPassword(passwordSize));

i.e. use the generation as a static method, and pass the requested size as a parameter.

StackExchange Code Review Q#31937, answer score: 7