How to build a class for comparing words in a lexical dictionary?

Question

I'm wanting to create a class that stores words in a set so that I can see if a word belongs to that set or not. I'm not wanting to build ever set every time I instantiate the class, so I'm using what I think is dependency injection. The following is a basic structure of the class. I'm a student learning about programming so using third party libraries isn't an option.

Is this an appropriate way to hard code my sets? Am I on the right track or should I be designing my class completely different from this?

This code is presented to demonstrate if my understanding of class design is correct or not.

public final class WordCollection {

    private final Set<String> chapter_types = new HashSet<>();
    private final Set<String> prepositions = new HashSet<>();

    public WordCollection(){
        buildChapterTypes();
        buildPrepositions();
    }

    public WordCollection(String collectionType) {
        if(collectionType.equals("chapters")) {
            buildChapterTypes();
        } else if(collectionType.equals("prepositions")) {
            buildPrepositions();
        }
    }

    private void buildChapterTypes() {
        chapter_types.add("Prologue");
        chapter_types.add("Section");
        chapter_types.add("Epilogue");
        chapter_types.add("Chapter");
    }

    public boolean isChapterType(String word) {
        return chapter_types.contains(word);
    }

    public void buildPrepositions(){
        prepositions.add("of");
        prepositions.add("for");
    }

    public boolean isPreposition(String word) {
        return prepositions.contains(word);
    }

}

Answer 1

I agree with what Christophe says in his answer. One thing I can't ignore is your claim to be using dependency injection. What you're doing is NOT dependency injection at all. You have your construction code (the build methods) mixed with your behavior code (the is methods). There is no way to override this construction code. This makes your code hard to change and hard to test. Dependency injection says you should ask for what you need, not look for, or build, what you need.

But if you want to do, what I think you want to do, your fundamental problem is that you need a different data structure.

I have one that will let you do this:

public static void main(String[] args) {
    WordCollection wc = new WordCollection(new MapOfSetsBuilder()
        .add("prologue", "chapters")
        .add("section", "chapters")
        .add("epilogue", "chapters")
        .add("chapter", "chapters")
        .add("of", "prepositions")
        .add("for", "prepositions")
        .add("smurf", "chapters", "prepositions", "articles", "verbs", "adverbs", "nouns")
        .build()
    );

    if (wc.word("prologue").isOfType("chapters")) {
        System.out.println("Whoo Hoo!");
    }
    if (wc.word("smurf").isOfType("nouns")) {
        System.out.println("Smurftastic!");
    }
    if (wc.word("smurf").isOfType("gargamel")) {
        System.out.println("Noo! Why? This is so wrong.");
    }
}

Outputs:

Whoo Hoo!
Smurftastic!

The structure? Map<String, Set<String>>

I'm mapping each word to it's type. A word might have more than one type so it maps to a set of types.

All you need is this:

//Construction class
class MapOfSetsBuilder {
    Map<String, Set<String>> result = new HashMap<>();
    public MapOfSetsBuilder add(String key, String... values) {
        Set setForKey = new HashSet<String>();
        setForKey.addAll(Arrays.asList(values));
        result.put(key, setForKey );
        return this;
    }
    public Map<String, Set<String>> build() {
        return result;
    }
}

//Behavior class
class WordCollection {
    Map<String, Set<String>> categories;

    WordCollection (Map<String, Set<String>> categories){
        this.categories = categories;
    }

    public TypeTest word(String word) {
        return new TypeTest(word);
    }

    class TypeTest {
        String word;
        TypeTest(String word) {
            this.word = word;
        }
        Boolean isOfType(String category) {
            return WordCollection.this.categories.get(word).contains(category);
        }
    }
}

I'm not wanting to build ever set every time I instantiate the class ...

OK ya lazy bum here's how to do that without shooting proper dependency injection in the face:

//Behavior class
class WordCollection {
    Map<String, Set<String>> categories;
    WordCollection(Map<String, Set<String>> categories) {
        this.categories = categories;
    }

    public TypeTest word(String word) {
        return new TypeTest(word);
    }

    class TypeTest {
        String word;

        TypeTest(String word) {
            this.word = word;
        }

        Boolean isOfType(String category) {
            return WordCollection.this.categories.get(word).contains(category);
        }
    }
    public static class Builder {
        public WordCollection buildDefault() {
            return new WordCollection(new MapOfSetsBuilder()
            .add("prologue", "chapters")
            .add("section", "chapters")
            .add("epilogue", "chapters")
            .add("chapter", "chapters")
            .add("of", "prepositions")
            .add("for", "prepositions")
            .add("smurf","chapters","prepositions","articles","verbs","adverbs","nouns")
            .build()
            );
        }
    }
}

Rather than a static factory method that can't be passed around as a reference in true dependency injection fasion I've opted for a static inner builder class. This is reminiscent of the Josh Bloch builder pattern but is less of a pain because of the MapOfSetsBuilder class helping out. This is invoked with:

WordCollection wc = new WordCollection.Builder().buildDefault();

Why not a simple static factory method? Because I'm on a static method hating kick. I just don't like being forced out of the instance mode when there is no good reason aside for the need for a little more typing.

Anyway, this last bit is not needed for dependency injection. It's just a convenience. It does nothing you couldn't have done in main and shared around. The constructor that takes the map is what is supporting dependency injection here. Done this way at least the WordCollection constructor can still take any stringy map of sets. Not just this default map.

Answer 2

If I understand well, the most important functions here are isChapter() and isPreposition(), to recognize known words and determine their category.

Hard-coding the sets like done here considerably reduces the use of your class in real word. So I'd suggest to externalize the hard coded builders:

• either construct the object providing the sets to be used at construction (you could use the builder design pattern for that)
• or provide an addPreposition() an addChapter() primitives and use an external helper class to fill them with hard coded values or, better, load them from a file

Finally, chapters and prepositions are only some lexical categories like preposition, article, verbs, adverbs, and nouns (chapter being a specialized subset of the more general noun). It could be sufficient for the purpose, but maybe you intend to add many more sets for many more lexical categories. In that case, I suggest to abstract further:

• make the category a separate class (or enum), change your interface to boolean is(Category,String) and Category getCtategory(String) and use one single map instead of an army of sets.
• or, strengthen your design with the principle of single responsibility: factor the categories' implementation out of your class and generalize them into lexical component (i.e. have one set for each category) and make your class an "assembly of components" (i.e. a kind of simplified flat composite), forwarding its incoming calls to its components.