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

The only thing better than being unique

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

Problem

is being first and unique.

Challenge:


Write a program which finds the first non-repeated character in a string.

Specifications:


The first argument is a path to a file.

The file contains strings, one per line.

Print out the first non-repeated character of each string.

Solution:

import java.io.File;
import java.io.FileNotFoundException;
import java.util.Map;
import java.util.LinkedHashMap;
import java.util.Scanner;

public class NoRepeat {
    public static void main(String[] args) throws FileNotFoundException {
        Scanner input = new Scanner(new File(args[0]));

        while (input.hasNextLine()) {
            System.out.println(
                retrieveFirstNonRepeatedLetter(input.nextLine())
            );
        }
    }

    public static char retrieveFirstNonRepeatedLetter(String input) {
        Map letters = new LinkedHashMap<>();
        char firstNonRepeated = ' ';

        for (int i = 0; i  entry : letters.entrySet()) {
            if (entry.getValue() == true) {
                firstNonRepeated = entry.getKey();
                break;
            }
        }

        return firstNonRepeated;
    }
}


This solution is \$O(n)\$ but I get the feeling that it's far from the best possible way to accomplish this.

I thought maybe I should set it to account for upper and lower cases, but it wasn't specified (and 100% passed with this implementation).

Tests:

public class MyFirstUnitTest {
    @Test
    void toothyTest() {
        assertEquals('h', NoRepeat.retrieveFirstNonRepeatedLetter("tooth"));
    }

    @Test
    void smellyTest() {
        assertEquals('d', NoRepeat.retrieveFirstNonRepeatedLetter("odor"));
    }
}

Solution

This is on top of @tim's excellent review.
I think this is a very good solution,
and a clever use of LinkedHashMap.
I can't think of a better or simpler algorithm.

Simplifications

This can be written simpler:

if (letters.containsKey(current)) {
    letters.put(current, false);
} else {
    letters.put(current, true);
}


Like this:

for (int i = 0; i < input.length(); i++) {
    char current = input.charAt(i);
    letters.put(current, !letters.containsKey(current));
}


Instead of comparing boolean expressions to true like this:

if (entry.getValue() == true) {


You can use them directly:

if (entry.getValue()) {


Unit tests

It's nice that you start adding unit tests. It will pay off!

To run your test cases in an IDE,
you need to make them public (at least I had to, in IntelliJ).

As a small extra tip,
to avoid long repeated calls like NoRepeat.retrieveFirstNonRepeatedLetter(...),
sometimes it can be practical to create a helper method, for example:

void assertFirstNonRepeatedLetter(char expected, String string) {
    assertEquals(expected, NoRepeat.retrieveFirstNonRepeatedLetter(string));
}

@Test
public void toothyTest() {
    assertFirstNonRepeatedLetter('h', "tooth"); 
}

@Test
public void smellyTest() {
    assertFirstNonRepeatedLetter('d', "odor"); 
}


Why fall back to firstNonRepeated = ' ' ?

I suggest to pay close attention to the specs.
It doesn't mention what should happen if there are no unique letters in the input.
Don't assume arbitrary fallback values.
The behavior is undefined, so it's kind of an illegal state.
It's a good move to throw an IllegalStateException in this case:

public static char retrieveFirstNonRepeatedLetter(String input) {
    Map letters = new LinkedHashMap<>();

    for (int i = 0; i  entry : letters.entrySet()) {
        if (entry.getValue()) {
            return entry.getKey();
        }
    }

    throw new IllegalStateException("No non-repeated characters");
}


And add a corresponding unit test to cover this:

@Test(expected = IllegalStateException.class)
public void nothingUnique() {
    NoRepeat.retrieveFirstNonRepeatedLetter("abab");
}

Code Snippets

if (letters.containsKey(current)) {
    letters.put(current, false);
} else {
    letters.put(current, true);
}
for (int i = 0; i < input.length(); i++) {
    char current = input.charAt(i);
    letters.put(current, !letters.containsKey(current));
}
if (entry.getValue() == true) {
if (entry.getValue()) {
void assertFirstNonRepeatedLetter(char expected, String string) {
    assertEquals(expected, NoRepeat.retrieveFirstNonRepeatedLetter(string));
}

@Test
public void toothyTest() {
    assertFirstNonRepeatedLetter('h', "tooth"); 
}

@Test
public void smellyTest() {
    assertFirstNonRepeatedLetter('d', "odor"); 
}

Context

StackExchange Code Review Q#83506, answer score: 6

Revisions (0)

No revisions yet.