gissuebot
commented
9 years ago Original comment posted by boppenheim@google.com on 2010-10-15 at 05:43 PM
What is the use case for having children fill a List instead of just having them return Lists?
In your example, why not:
List<String> list = Lists.newArrayList(); for(ListFiller filler : fillers) { filler.fillList(list); } Collections.sort(list, String.CASE_INSENSITIVE_ORDER); ImmutableList<String> sortedImmutableList = ImmutableList.copyOf(sort);
It seems to me like adding sorting to ImmutableList.Builder would not be in line with the purpose of the class. While adding it might save a line of code, I think its worthwhile to keep sorting a separate behavior. IMHO, it would be nice if Collections.sort returned list as a convenience so you could do ImmutableList.copyOf(Collections.sort(list, comparator)); Although, I wonder if this would mislead people who don't read the javadoc to believe that Collections.sort returns a sorted copy.
Original issue created by ogregoire on 2010-10-15 at 03:31 PM
Currently if you want your users to fill your lists, you just create an interface:
public interface ListFiller { public void fillList(List<String> builder); }
Then you use it like this:
List<String> builder = Lists.newArrayList(); for (ListFiller lf: fillers) { List<String> tmp = Lists.newArrayList(); lf.fillList(tmp); builder.addAll(tmp); } Collections.sort(builder, String.CASE_INSENSITIVE_ORDER); ImmutableList<String> sortedImmutableList = ImmutableList.copyOf(builder);
But I find this extremely heavy for a such simple task. So I suggest creating a new builder for the ImmutableList that would automatically sort all elements at the build time.
public interface ListFiller { public void fillList(ImmutableList.Builder<String> builder); }
That would be used this way:
ImmutableList.Builder<String> builder = ImmutableList.builder(String.CASE_INSENSITIVE_ORDER); for (ListFiller lf: fillers) { lf.fillList(builder); } ImmutableList<String> sortedImmutableList = builder.build();
Much smaller, much more intuitive, much more readable.
I think that internally there should be a SortingBuilder but the user never sees it, so that would make this SortingBuilder extending the normal Builder.