Sean Patrick Floyd
Sean Patrick Floyd

Reputation: 298908

Adding and removing items to a Guava ImmutableList

In Guava, is there an efficient way to add or remove items to an ImmutableList (creating new Lists in the process, of course).

The simplest way I can come up with is this:

private ImmutableList<String> foos = ImmutableList.of();

public void addFoo(final String foo) {
    if (this.foos.isEmpty()) {
        foos = ImmutableList.of(foo);
    } else {
        foos = ImmutableList.<String>builder().addAll(foos).add(foo).build();
    }
}

public void removeFoo(final String foo) {
    final int index = this.foos.indexOf(foo);
    if (index > -1) {
        final Builder<String> builder = ImmutableList.<String>builder();
        if (index > 0) builder.addAll(this.foos.subList(0, index));
        final int size = this.foos.size();
        if (index < size - 1) builder.addAll(this.foos.subList(index+1, size));
        this.foos = builder.build();
    }
}

What I would like to avoid doing is this:

public void removeFoo(final String foo) {
    final ArrayList<String> tmpList = Lists.newArrayList(this.foos);
    if(tmpList.remove(foo))this.foos=ImmutableList.copyOf(tmpList);
}

But unfortunately it is so much simpler than any Guava-only method I can think of. Have I missed something?

Upvotes: 16

Views: 20472

Answers (2)

maaartinus
maaartinus

Reputation: 46422

The ConcurrentModificationException isn't really related to concurrency and synchronization. Accessing a mutable List concurrently might corrupt it and/or throw the exception (be prepared to all 3 possibilities). You code can't fail this way, but with multithreading it doesn't work either:

  • Without synchronization and without foos being volatile, there's no guarantee that another thread will ever see the changes you've made.
  • Even with volatile, it can happen that some changes get lost, e.g., when two threads add an item to foos, both of them can start with the original value and then the one writing last wins (and only its item gets added).

The code you're trying to avoid is nothing to be avoided.

  • "I have to create superfluous intermediate collections" - yes, but there's no free lunch:
    • determine the size of the result in advance, which means an additional iteration through the whole list
    • or allocate a large enough array and copy the needed range in the resulting list
    • or allocate a large enough array and use only part of it (saving time and wasting memory)
    • or create an immutable view (saving both time and memory, but possibly losing time later)
  • AFAIK Frank's answer implements the first possibility, which is fine if the predicate is fast.
  • "I have to mix java.util Collections with guava ImmutableCollections, while I'd like to stick to one paradigm." - yes, but for mutating a collection a mutable collection is needed. The ImmutableList.Builder covers just the most common cases allowing to handle them in a compact way.

You might want to have a look at persistent collections, which are optimized for such operations. However, you shouldn't expect e.g. the persistent list to be as fast as an ArrayList or ImmutableList.

Upvotes: 7

Frank Pavageau
Frank Pavageau

Reputation: 11715

You can remove by filtering, which doesn't create an intermediate ArrayList or builder, and only traverses the list once:

public void removeFoo(final String foo) {
    foos = ImmutableList.copyOf(Collections2.filter(foos,
            Predicates.not(Predicates.equalTo(foo)));
}

For adding, I don't see a better solution.

Upvotes: 19

Related Questions