Naming Vegetable Factories

Posted on

Problem

Given the code below, I’m curious to know if you’d have better naming suggestions for types:

  • VegetablePresentersFactory
  • VegetablePresenterFactory

It bothers me a bit that their names only differ by a single character (‘s’), and that this character is located in the middle of the name.

class VegetablesApp {
    private final VegetablePresentersFactory vegetablePresentersFactory; // gets injected

    public void displayVegetablePresenters(AllVegetables all) {
        List<VegetablePresenter> presenters = vegatablePresentersFactory.create(all);

        displayPresenters(presenters);
    }
}

class VegetablePresentersFactory {
    private final VegetablePresenterFactory vegetablePresenterFactory;

    public List<VegetablePresenter> create(AllVegetables all) {
        List<VegetablePresenter> presenters = new ArrayList<>();

        list.add(vegetablePresenterFactory.create(all.getTomato()));
        list.add(vegetablePresenterFactory.create(all.getLettuce()));
        list.add(vegetablePresenterFactory.create(all.getBacon()));

        return presenters;
    }
}

interface VegetablePresenterFactory {
    VegetablePresenter create(Vegetable vegetable);
}

Solution

How about replacing “s” of plural with the word “List”:

interface VegetablePresenterListFactory {    
    List<VegetablePresenter> create(AllVegetables allVegetables);
}

Like you said, the s in the middle is indeed a problem.
The difference is too small: it can lead to errors, and it hurts readability.

The same is true for name vs names in this code snippet:

void printNames(List<String> names) {
    for (String name : names) {
        // ...
    }
}

Although errors may sound unlikely because name and names are completely different types,
it can still happen when there are methods with the same name that happen to work on both objects,
for example isEmpty.

In such cases I use the suffix List to disambiguate, so nameList.
It’s true that the term “List” is a bit redundant.
But it’s better to be redundant than ambiguous,
so the little redundancy is justified by the improved readability.

UPDATE

You mentioned in the comment that “List” is a bit unfortunate if in the future you change the interface to use a Set instead of a List.
I think that’s a moot point.
Changing the interface is a big deal,
and must be done carefully.
If you change the method to return a Set instead of a List,
I think you should be careful enough to notice that the interface name should change as well.
In terms of migrating users of the API,
this should not be a significant additional burden.

That said, I do have an alternative offering, using the word “Items” instead of “List”:

interface VegetablePresenterItemsFactory {    
    List<VegetablePresenter> create(AllVegetables allVegetables);
}

The good thing about “Items” is that it’s less technical.
It’s vague enough to be flexible, but not too vague to be meaningless.
Perhaps there are even better synonyms, you get the idea.

“The two hardest things in computing are cache invalidation, naming things, and off-by-one errors.”

There’s a particular art to naming things well, but like any art, appreciation is subjective. In this case, I’d strongly prefer VegetablePresenterFactory to match the fact that it is a factory for generating VegetablePresenter objects. The fact that it does so en masse and gives you a list of many at a time is not important enough to be in the name of the class; a factory could have multiple factory methods that vary whether they generate one or many VegetablePresenters.

There could be an argument for not embedding the list behaviour into the Factory. It may make your current use-case simpler, but in future it makes the Factory hard to use if you wish to only generate one object at a time. Having a simple loop when calling the factory to create multiple objects will still result in fairly clean, concise and easy-to-understand code. It also avoids issues associated with whether you should accept and/or return a List, Set, or just a Collection.

void printNames(List<String> names) {
    for (String name : names) {
        // ...
    }
}

I’m in disagreement with @janos over the use of the rather similar variable names here. Sure, they’re quite similar, but only List<String> names is visible outside of the function as part of the method signature and that’s quite valid and descriptive. The String name within the for..each loop is very local to where it is being used and the opportunity for confusion is slim. Using the plural for a collection and the singular within the scope of a for..each loop over such a collection gives very natural language and makes the code nice to read.

Leave a Reply

Your email address will not be published. Required fields are marked *