HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Command line password generator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
commandgeneratorlinepassword

Problem

I built a random password generator in Java which is meant to be run from the command line. It accepts options for:

  • Length



  • Upper case letters



  • Lower case letters



  • Special characters



  • Numbers



The project is on BitBucket.

Example program execution :

java -jar program.jar -length 12 -number -upper -lower -special


This generates a 12 character long password with upper and lower case letters, numbers, and special characters.

What I think :

  • I find my Parser class to be quite messy. It would be hard to add a new parameter with different properties. How would you do it?



Parser.java

```
package passManager;

import java.util.ArrayList;
import java.util.Arrays;

public class Parser {
public static ArrayList makeArgList(String args[]) {

ArrayList argList = new ArrayList();
boolean allArgsValid = true;

// Checks if each arg is valid
for (int i = 0; i < args.length && allArgsValid; i++) {

// If it starts with - and is not in the list of parameters, bad
if (args[i].startsWith("-") && !isInArgumentsList(args[i], Constants.VALID_ARGS)) {
System.out.println("Invalid parameter: " + args[i] + ". Exiting.");
allArgsValid = false;

// If it's not a parameter and it isn't a number, bad
} else if (!isInArgumentsList(args[i], Constants.VALID_ARGS) && !args[i].matches("^\\d+$")) {
System.out.println("Invalid parameter: " + args[i] + ". Exiting.");
allArgsValid = false;

// Verify the length parameter
} else if (args[i].equals("-length")) {

// If length is the only parameter (-length or -length 5 only is
// bad)
if (i == 0 && (i + 1 == args.length || i + 2 == args.length)) {
System.out.println("Length cannot be the only parameter. Exiting.");
allArgsValid = false;

// If what comes after -length is 0 or not a number
} else if (!args[i + 1].m

Solution

Command-line parsing

Instead of writing your bespoke methods to parse command-line options, consider using a library that can do that for you quite effortlessly. A quick Google search throws up the likes of Apache Commons CLI and JCommander.

Interfaces over implementations

You are passing around ArrayList as a method argument or return type, but all your methods need to know is just the List interface. Making this change is preferred as it allows you to swap different List implementations if required. E.g. if you are writing unit tests that can rely on Arrays.asList("some", "text") to give you a List, instead of having to create an ArrayList and then populate it.

static fields

Generator.WORKING_SET is a static StringBuilder, but it is used in a decidedly non-static way. You are building a String to generate your characters from, so this String should reside within the method. Leaving it as a static field allows it to be abused quite easily, e.g. concurrent calls made to modify WORKING_SET with different acceptable values may not generate the right randomized results.

Context

StackExchange Code Review Q#129478, answer score: 2

Revisions (0)

No revisions yet.