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

Find meaningful words from the input string

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

Problem

Given a string find the number of meaningful words which could be formed from the string
for eg. programmerit forms:

pro+gram+merit
program+merit
programmer+it
pro+grammer+it


Looking for code review optimizations and best practices. Also hoping for suggestions for complexity better than \$O(n^2)\$, where n is the number of characters.

public final class DictionaryValidWords {

    private static final Set dictionary = new TreeSet();
    static {
        dictionary.add("this");
        dictionary.add("his");
        dictionary.add("is");
        dictionary.add("awe");
        dictionary.add("we");
        dictionary.add("some");
        dictionary.add("awesome");
        dictionary.add("foo");
        dictionary.add("bar");
    }

    private DictionaryValidWords() {}

    /**
     * Returns set of valid words given an input string.
     * It eliminates duplicates.
     * 
     * @param str   The input string whose valid words need to be found out.
     * @return      List of valid words nested in the string.
     */
    public static Set findValidStrings(String str) {
        if (str.length() ==  0) {
            throw new IllegalArgumentException("Strings of length 0 are illegal");
        }

        final Set validWords = new HashSet(); 
        for (int i = 0; i  expectedSet = new HashSet(Arrays.asList("awe", "is", "his", "awesome", "some", "this", "we"));
        assertEquals(expectedSet, DictionaryValidWords.findValidStrings("thisisawesome"));
    }
}

Solution

I only have one minor objection for the moment...

...

...
Why are you not using dependency injection??

OK, it turned out to be a major one. (I did consider using caps-lock!)

Consider a situation where you would want to do this operation for English words, and then for French words. Instead of using all things static, make a public constructor

private final Set dictionary;

public DictionaryValidWords(Set dictionary) {
    this.dictionary = dictionary;
}


You might consider this as "outside the core part of the code" but it's really not. And it is a very important change to make, class-design wise and best-practices wise.

You could then use a DictionaryFactory class where you could have methods (static or non-static, as you prefer), for exampleDictionary (your current dictionary), english, french, etc...

Code Snippets

private final Set<String> dictionary;

public DictionaryValidWords(Set<String> dictionary) {
    this.dictionary = dictionary;
}

Context

StackExchange Code Review Q#49366, answer score: 11

Revisions (0)

No revisions yet.