patternjavaMinor
The only thing better than being unique
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:
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:
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
I can't think of a better or simpler algorithm.
Simplifications
This can be written simpler:
Like this:
Instead of comparing boolean expressions to
You can use them directly:
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
As a small extra tip,
to avoid long repeated calls like
sometimes it can be practical to create a helper method, for example:
Why fall back to
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
And add a corresponding unit test to cover this:
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.