During code review, a colleague of mine looked at this piece of code:
public List<Item> extractItems(List<Object[]> results) {
return Lists.transform(results, new Function<Object[], Item>() {
@Override
public Item apply(Object[] values) {
...
}
});
}
He suggests changing it to this:
public List<Item> extractItems(List<Object[]> results) {
return Lists.transform(results, getTransformer());
}
private Function<Object[], Item> transformer;
private Function<Object[], Item> getTransformer() {
if(transformer == null) {
transformer = new Function<Object[], Item>() {
@Override
public Item apply(Object[] values) {
...
}
};
}
return transformer;
}
So we are looking at taking the new Function() construction, and moving it over to be a member variable and re-used next time.
While I understand his logic and reasoning, I guess I'm not sold that I should do this for every possible object that I create that follows this pattern. It seems like there would be some good reasons not to do this, but I'm not sure.
What are your thoughts? Should we always cache duplicately created objects like this?
UPDATE
The Function is a google guava thing, and holds no state. A couple people have pointed out the non-thread-safe aspect of this change, which is perfectly valid, but isn't actually a concern here. I'm more asking about the practice of constructing vs caching small objects, which is better?
