Creating and searching a dictionary of passwords and MD5 hashes

In this program, I have been asked to read an existing textfile (1324passlist) containing a list of passwords, then create a dictionary file with both the password and MD5 hash of the password on the same line. It should also have the user input a password hash, search the created dictionary file and print the corresponding password, if it exists.
I'm not a programmer and it's not pretty, but it runs according to the assignment guidelines.

I would like advice on where I could tidy up and write the code more efficiently.

```
public class main
{
public static void searchTextFile(String hash) throws IOException
{
File fileName = new File("dictionary.txt");
Scanner scan = new Scanner(fileName);

String line = scan.nextLine();
StringTokenizer st = new StringTokenizer(line, " ");

while(st.hasMoreTokens())
{
String word1 = st.nextToken();
String word2 = st.nextToken();

if(word2.equals(hash))
{
System.out.println(word1);
break;
}

if(!st.hasMoreTokens())
{
System.out.println("Hash not found.");
}
}
}

public static String getMd5(String pInput)
{
try
{
MessageDigest lDigest = MessageDigest.getInstance("MD5");
lDigest.update(pInput.getBytes());
BigInteger lHashInt = new BigInteger(1, lDigest.digest());
return String.format("%1$032X", lHashInt);
}
catch(NoSuchAlgorithmException lException)
{
throw new RuntimeException(lException);
}
}

public static void findPassword(String fileName, String hash)
{
try
{
Scanner scanner = new Scanner(new File(fileName));
List pwAndHash = new ArrayList();

while(scanner.hasNext())
{
pwAndHash.add(scanner.next());

Some comments that don't relate to efficiency:

Exceptions and exception handling. In a couple of places you are SQUASHING exceptions; e.g.

catch(FileNotFoundException e)
    {
    }

So, if we can't open the password file, we effectively ignore the error, and ... behave exactly as if the password was correct.

catch( Exception e ) { }

This is even worse. You are now squashing all exceptions, including any NPEs, class cast exceptions, etc caused by bugs in your code or (hypothetically) in library code you are calling.

Lesson #1: - Do not squash exceptions. Any exception that is out of the ordinary should result in at least an error message to report the problem. And if your code is not anticipating the exception, then you should print / log a stack trace.

This is bad:

try {
        String hash1 = hash.toUpperCase();
        int hashIndex = pwAndHash.indexOf(hash1);
        int passwordIndex = hashIndex - 1;
        System.out.println(pwAndHash.get(passwordIndex));
    } catch(ArrayIndexOutOfBoundsException a) {
        System.out.println("The hash was not found.");
    }

Reasons:

-
You are expecting that the ArrayIndexOutOfBoundsException will be thrown in this: pwAndHash.get(passwordIndex). But that statement won't throw that exception. If the index is wrong is wrong it throws IndexOutOfBoundsException.

-
Supposing it did throw that exception, you still have the problem that other statements in the block could throw that exception ... for a completely different reason. For example, if you were using your own List implementation class, it could be due to an unexpected bug in that class.

-
Throwing and catching an exception in Java is relatively expensive. In this case, there is a simpler, cleaner, more efficient solution:

String hash1 = hash.toUpperCase();
int hashIndex = pwAndHash.indexOf(hash1);
if (hashIndex == -1) {
    System.out.println("The hash was not found.");
} else {
    int passwordIndex = hashIndex - 1;
    System.out.println(pwAndHash.get(passwordIndex));
}

Lesson #2: Be really careful when catching unchecked exceptions to make sure that you don't hide bugs ... or introduce new ones by catching the wrong exception.

Lesson #3: Don't use exceptions for normal control flow ... unless it really simplifies things. (Some people would say never use exceptions for control flow, but there are limited cases where it is (IMO) justified to use them.)

Stylistic:

-
Indent your code properly, and make sure that you are consistent in your use of TAB characters. (Ideally reconfigure your IDE so that it doesn't use them at all!).

-
Be consistent about line breaks, and embedded whitespace.

-
I'd recommend NOT putting opening braces for code blocks on a new line. It wastes vertical screen space ... and goes against Best Practice as set out in the Sun Java Style Guide (see below).

When I am marking student code, they will get severely penalized for poor style. And in a professional situation, poor style results in rejection of code on quality grounds. When you write code, you write it for other people to read. If it is not readable, it is not fit for purpose. Period.

To those who think that placement of braces is a personal preference, the following is quoted from the Sun Java Style Guide:

7.2 Compound Statements

Compound statements are statements that contain lists of statements enclosed in braces "{ statements }". See the following sections for examples.

• The enclosed statements should be indented one more level than the compound statement.

• The opening brace should be at the end of the line that begins the compound statement; the closing brace should begin a line and be indented to the beginning of the compound statement.

• Braces are used around all statements, even single statements, when they are part of a control structure, such as an if-else or for statement. This makes it easier to add statements without accidentally introducing bugs due to forgetting to add braces.

And the Guide has many examples to illustrate what they mean by that.

StackExchange Code Review Q#18004, answer score: 5