# Number Plate Generation Program

I wrote a number plate generation program. The program generates number plates for a car registered with a specific Memory Tag and on a specific date.

Number plates use the following format: (2 letter Memory Tag)(2 digit Age Identifier)(space)(3 Random Letters)

The Memory Tag is already given.

Age identifiers are the last two digits of the year if the date given is between March and August, or the last two digits plus 50 if the date given is between September and February (registration years can be thought of starting in March). So, for 12th of March 2018, it is “18” and for the 27th of February 2019, it is “68” (not “69”).

The 3 random letters can be any except “I” and “Q”.

The number plates generated are stored in a text file, each time the program is run, it checks if the number plate already exists. If it does, the 3 random letters are generated again.

As an example, if given the memory code “PJ” and the date 21/10/2025 than a valid output could be “PJ75 STC”.

This is one of the first programs I've written by myself and I want to get used to good programming practices, is there anything here that is bad practice? Anything that could be improved?

            public static String numberPlate(String tag, Date date) {

String returnS = tag; //return String

SimpleDateFormat monthFormat = new SimpleDateFormat("MM");
int month = Integer.parseInt(monthFormat.format(date));

SimpleDateFormat yearFormat = new SimpleDateFormat("YYYY");
String year = yearFormat.format(date);
String age;

if(month >= 3 && month <= 8){ //If the month is between March and August
age = year.substring(2); //get last two digits of the Year
}
else{
age = String.valueOf(50 + Integer.parseInt(year.substring(2)));
}

returnS = returnS + age + " "; //add Age identifier

//generate 3 random characters
Random r = new Random();
int counter=0;
do {
char c = (char) (r.nextInt(26) + 'a');
if(c!='i' && c!='q') {
returnS = returnS + Character.toUpperCase(c); //adds random character to returnS
counter++; //keeps track of how many characters have been generated
if (counter == 3) {
try { //Once 3 characters have been generated, check in the textfile if this plate exists
FileInputStream fstream = new FileInputStream("plates.txt");
String strLine;

while ((strLine = br.readLine()) != null) {
if (strLine.equals(returnS)) {
returnS = returnS.substring(0, 5); //returnS wipes away the 3 characters if the plate exists
counter = 0;
}
}
br.close();
} catch (Exception e) {
e.printStackTrace();
}
}
}
}
while (counter <3); //runs until 3 characters have been generated, and the plate is unique

//write the plate into the text file
File log = new File("plates.txt");
try{
FileWriter fileWriter = new FileWriter(log, true);
BufferedWriter bufferedWriter = new BufferedWriter(fileWriter);
bufferedWriter.write(returnS);
bufferedWriter.newLine();
bufferedWriter.close();
} catch(IOException e) {
}

System.out.println(returnS);
return returnS;

}


### General formatting

Keeping code nicely formatted makes it easier to read, and therefore easier to work on. This is especially important when you work on a project with multiple people, where other people may make changes to code you wrote originally, or vice versa.

Some of your lines are indented too much, specifically the first line and the final try-catch block. You are also missing spaces in some places; for example, if(c!='i' && c!='q') { should be if (c != 'i' && c != 'q') {.

There should be an option in your IDE to auto-format your code with a simple keyboard command, or even every time you save.

### do-while loops

In general, I suggest using while loops rather than do-while loops, unless you have a specific reason not to. do-while isn't wrong, per se, but it is used far less often and may catch some people off-guard.

The exception is if you join a team where everybody uses do-while loops all the time, and your code would stick out if you didn't use them. "When in Rome..."

### try-with-resources

Remembering to call .close() on your readers and writers is always a bit of a hassle. You also run the risk that you will hit an exception somewhere along the way and never reach the .close() at all.

The solution is to use the try-with-resources statement. Java will auto-close the resources when the code exits the try-block, no matter how that happens.

//Once 3 characters have been generated, check in the textfile if this plate exists
try (FileInputStream fstream = new FileInputStream("plates.txt");

String strLine;

while ((strLine = br.readLine()) != null) {
if (strLine.equals(returnS)) {
returnS = returnS.substring(0, 5); //returnS wipes away the 3 characters if the plate exists
counter = 0;
}
}
}


### Big loop structure

1. Generate a suffix of three random letters and append it to the plate string
2. Check the plate string for uniqueness against an external file

These tasks are currently intertwined in a slightly confusing way; the second task appears to be a sub-operation of the first task, even though it is really just waiting for the first task to finish.

It would be clearer to structure the two tasks as separate operations inside a larger, "neutral" loop:

while (true) {

// Generate a random suffix of length 3
int counter = 0;
String randomSuffix = "";
while (counter < 3) {
char c = (char) (r.nextInt(26) + 'a');
if (c != 'i' && c != 'q') {
randomSuffix += Character.toUpperCase(c);
counter++;
}
}

// Add suffix to plate and check for uniqueness
final String possiblyUniquePlate = returnS + randomSuffix;
boolean isUnique = true;
try (FileInputStream fstream = new FileInputStream("plates.txt");
String strLine;
while ((strLine = br.readLine()) != null) {
if (strLine.equals(possiblyUniquePlate)) {
isUnique = false;
break;
}
}
} catch (Exception e) {
// We'll talk about exception-handling later...
e.printStackTrace();
isUnique = false; // We can't verify uniqueness if we can't read the file
}

// If plate is confirmed unique, set as return value and exit loop
if (isUnique) {
returnS = possiblyUniquePlate;
break;
}
}


...or better yet, break the tasks out into separate functions:

private static String generateRandomSuffix(Random r) {
// Generate a random suffix of length 3
int counter = 0;
String randomSuffix = "";
while (counter < 3) {
char c = (char) (r.nextInt(26) + 'a');
if (c != 'i' && c != 'q') {
randomSuffix += Character.toUpperCase(c);
counter++;
}
}
return randomSuffix;
}

private static boolean isPlateUnique(final String possiblyUniquePlate) {
boolean isUnique = true;
try (FileInputStream fstream = new FileInputStream("plates.txt");
String strLine;
while ((strLine = br.readLine()) != null) {
if (strLine.equals(possiblyUniquePlate)) {
isUnique = false;
break;
}
}
} catch (Exception e) {
// We'll talk about exception-handling later...
e.printStackTrace();
isUnique = false; // We can't verify uniqueness if we can't read the file
}
return isUnique;
}

Random r = new Random();
// runs until 3 characters have been generated, and the plate is unique
while (true) {
String randomSuffix = generateRandomSuffix(r);

// Add suffix to plate and check for uniqueness
final String possiblyUniquePlate = returnS + randomSuffix;
boolean isUnique = isPlateUnique(possiblyUniquePlate);

// If plate is confirmed unique, set as return value and exit loop
if (isUnique) {
returnS = possiblyUniquePlate;
break;
}
}


### Random char generation

If you're always going to capitalize the letters, and you're only using ASCII letters anyway, why not start with capital letters? 'A' instead of 'a'.

char c = (char) (r.nextInt(26) + 'A');
if (c != 'I' && c != 'Q') {
randomSuffix += c;
counter++;
}


Next, I don't like having 'I' and 'Q' in that if-statement. Those are basically "magic numbers", and should be replaced with one or more named constants.

Now, we could do something with an unmodifiable set, like this:

private static final Set<Character> INVALID_SUFFIX_CHARS = Set.of('I', 'Q');

while (counter < 3) {
char c = (char) (r.nextInt(26) + 'A');
if (!INVALID_SUFFIX_CHARS.contains(c)) {


...but I think it makes more sense to approach the problem from a "whitelist" perspective rather than a "blacklist" perspective. There are currently 149,186 characters in Unicode, and all but 24 of them are invalid for our purposes.

So, I would build an immutable list of valid characters, and then randomly select 3 from that list:

// I and Q are excluded
private static final List<Character> VALID_SUFFIX_CHARS = List.of(
'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'J', 'K', 'L', 'M',
'N', 'O', 'P', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z');

private static String generateRandomSuffix(Random r) {
// Generate a random suffix of length 3
String randomSuffix = "";
for (int i = 0; i < 3; i++) {
char c = VALID_SUFFIX_CHARS.get(r.nextInt(VALID_SUFFIX_CHARS.size()));
randomSuffix += c;
}
return randomSuffix;
}


### Exception handling

You have two catch blocks in your code where you catch exceptions:

} catch (Exception e) {
e.printStackTrace();
}

} catch(IOException e) {
}


Catching a generic Exception instead of more specific exceptions is a code smell. So is "swallowing" an exception—catching an exception and then doing nothing with it. There are rare cases when you may want to do these things, but if you can't give an specific explanation for why you are doing them, then you shouldn't.

The problem with the first block in particular—the one tied to reading the file for the uniqueness check—is that the loop continues after the exception is caught. If the exception was thrown by, say, new FileInputStream("plates.txt"), that means counter is still at 3, so the big loop will end and the program will proceed to the write phase. This could result in a duplicate license plate being written to the file, since the write might succeed even if the read failed.

If you can't write to the file, then the method cannot succeed in its task. If you can't read the file to check the license plate's uniqueness, then it is not safe to write to the file, and so the method cannot succeed in its task.

In the real world, your method would be part of a larger program that would take inputs from a user, or perform tasks on a timer. There would be code "above" this method that would be responsible for calling it. In that case, you would want your method to notify that "higher" code whenever it failed. The "higher" code would be responsible for deciding what to do next: trying again, notifying the user, etc.

So, when your method triggers an exception that blocks it from doing its task, the logical course of action is to have your method throw its own exception to the code "above" it.

This could be done by not catching exceptions at all, and simply letting the code above you deal with it:

public static String numberPlate(String tag, Date date) throws IOException {


Alternatively, you could define a new custom exception class, and wrap all caught exceptions in the new class before re-throwing them:

public static class LicenseGenerationException extends Exception {
private static final long serialVersionUID = 1L;

super(cause);
}
}

public static String numberPlate(String tag, Date date) throws LicenseGenerationException {

} catch (IOException e) {

First, you should definitely add a break to the subloop that goes through the file to check for uniqueness. As it's currently written, it always checks the candidate string against every line in the file, even if it finds a duplicate on the first line.
I also see that, whenever a string fails the uniqueness check, we close the readers for the file, and have to create new readers and start again for the next candidate. This is rather slow, relatively speaking. (It's still pretty fast from an human point of view.) As an alternative, you could read in the file at the start of the method, and store its contents as a HashSet<String>, if you have enough memory to do that comfortably. It is much faster to check uniqueness against a HashSet. Better yet, you could wait until you've constructed the prefix for your new plate, and then the set would only need to hold the strings from the file that start with that prefix.