문제

In the smell Data Class as Martin Fowler described in Refactoring, he suggests if I have a collection field in my class I should encapsulate it.

The pattern Encapsulate Collection(208) says we should add following methods:

  • get_unmodified_collection
  • add_item
  • remove_item

and remove these:

  • get_collection
  • set_collection

To make sure any changes on this collection need go through the class.

Should I refactor every class which has a collection field with this pattern? Or it depends on some other reasons like frequency of usage?

I use C++ in my project now.

Any suggestion would be helpful. Thanks.

도움이 되었습니까?

해결책

These are well formulated questions and my answer is:

Should I refactor every class which has a collection field with this pattern?

  • No, you should not refactor every class which has a collection field. Every fundamentalism is a way to hell. Use common sense and do not make your design too good, just good enough.

Or it depends on some other reasons like frequency of usage?

  • The second question comes from a common mistake. The reason why we refactor or use design pattern is not primarily the frequency of use. We do it to make the code more clear, more maintainable, more expandable, more understandable, sometimes (but not always!) more effective. Everything which adds to these goals is good. Everything which does not, is bad.

You might have expected a yes/no answer, but such one is not possible here. As said, use your common sense and measure your solution from the above mentioned viewpoints.


I generally like the idea of encapsulating collections. Also encapsulating plain Strings into named business classes. I do it almost always when the classes are meaningful in the business domain.

I would always prefer

public class People {
    private final Collection<Man> people;
    ... // useful methods
}

over the plain Collection<Man> when Man is a business class (a domain object). Or I would sometimes do it in this way:

public class People implements Collection<Man> {
    private final Collection<Man> people;

    ... // delegate methods, such as
    @Override
    public int size() {
        return people.size();
    }
    @Override
    public Man get(int index) {
        // Here might also be some manipulation with the returned data etc.
        return people.get(index);
    }
    @Override
    public boolean add(Man man) {
        // Decoration - added some validation
        if (/* man does not match some criteria */) {
            return false;
        }
        return people.add(man);
    }

    ... // useful methods
}

Or similarly I prefer

public class StreetAddress {
    private final String value;
    public String getTextValue() { return value; }
    ...
    // later I may add more business logic, such as parsing the street address
    // to street name and house number etc.
}

over just using plain String streetAddress - thus I keep the door opened to any future change of the underlying logic and to adding any useful methods.

However, I try not to overkill my design when it is not needed so I am as well as happy with plain collections and plain Strings when it is more suited.

다른 팁

I think it depends on the language you are developing with. Since there are already interfaces that do just that C# and Java for example. In C# we have ICollection, IEnumerable, IList. In Java Collection, List, etc.

If your language doesn't have an interface to refer to a collection regarless of their inner implementation and you require to have your own abstraction of that class, then it's probably a good idea to do so. And yes, you should not let the collection to be modified directly since that completely defeats the purpose.

It would really help if you tell us which language are you developing with. Granted, it is kind of a language-agnostic question, but people knowledgeable in that language might recommend you the best practices in it and if there's already a way to achieve what you need.

The motivation behind Encapsulate Collection is to reduce the coupling of the collection's owning class to its clients.

Every refactoring tries to improve maintainability of the code, so future changes are easier. In this case changing the collection class from vector to list for example, changes all the clients' uses of the class. If you encapsulate this with this refactoring you can change the collection without changes to clients. This follows on of SOLID principles, the dependency inversion principle: Depend upon Abstractions. Do not depend upon concretions.

You have to decide for your own code base, whether this is relevant for you, meaning that your code base is still being changed and has to be maintained (then yes, do it for every class) or not (then no, leave the code be).

라이센스 : CC-BY-SA ~와 함께 속성
제휴하지 않습니다 StackOverflow
scroll top