How to correctly copy collections with objects in Java

Question

I have a method (List<Book> getListWithPrefixInName(List<Book> books)) that accepts a collection as input and performs a transformation with each collection object. And I do not want these changes to be reflected on the transferred collection, just to return the modified collection. Because I need my original collection.

Accordingly, I thought to just copy by creating a new collection:

List<Book> clone = new ArrayList<>(books);

But how am I wrong... My code:

public class App {

    public static final String PREFIX = "PREFIX";

    public static void main(String[] args) {
        List<Book> books = new ArrayList<>();
        books.add(Book.of(1L, "The Catcher in the Rye"));
        books.add(Book.of(2L, "The Green Mile"));
        List<Book> booksWithPrefix = getListWithPrefixInName(books);

        for (Book book : books) {
            if (book.getName().contains(PREFIX)) {
                System.out.println(String.format("original book: '%s' have been changed", book));
            }
        }
    }

    public static List<Book> getListWithPrefixInName(List<Book> books) {
        List<Book> clone = new ArrayList<>(books);  // I thought that this cloning would be enough
        return clone.stream()
                .peek(b -> b.setName(PREFIX + ": " + b.getName()))
                .collect(toList());
    }
}

class Book {
    private Long id;
    private String name;

    private Book(Long id, String name) {
        this.id = id;
        this.name = name;
    }

    public static Book of(Long id, String name) {
        return new Book(id, name);
    }

    public Long getId() {
        return id;
    }

    public void setId(Long id) {
        this.id = id;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    @Override
    public String toString() {
        final StringBuilder sb = new StringBuilder("Book{");
        sb.append("id=").append(id);
        sb.append(", name='").append(name).append('\'');
        sb.append('}');
        return sb.toString();
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Book book = (Book) o;
        return Objects.equals(id, book.id) &&
                Objects.equals(name, book.name);
    }

    @Override
    public int hashCode() {
        return Objects.hash(id, name);
    }
}

And accordingly my entire original collection has been changed, it's terrible.

So I had to use deep cloning through a third-party library:

import org.apache.commons.lang3.SerializationUtils;

public class App {

    public static final String PREFIX = "PREFIX";

    public static void main(String[] args) {
        List<Book> books = new ArrayList<>();
        books.add(Book.of(1L, "The Catcher in the Rye"));
        books.add(Book.of(2L, "The Green Mile"));
        List<Book> booksWithPrefix = getListWithPrefixInName(books);

        for (Book book : books) {
            if (book.getName().contains(PREFIX)) {
                System.out.println(String.format("original book: '%s' have been changed", book));
            }
        }
    }

    public static List<Book> getListWithPrefixInName(List<Book> books) {
        return books.stream()
                .map(b -> SerializationUtils.clone(b))  // deep clone
                .peek(b -> b.setName(PREFIX + ": " + b.getName()))
                .collect(toList());
    }
}

class Book implements Serializable {
    private Long id;
    private String name;

    private Book(Long id, String name) {
        this.id = id;
        this.name = name;
    }

    public static Book of(Long id, String name) {
        return new Book(id, name);
    }

    public Long getId() {
        return id;
    }

    public void setId(Long id) {
        this.id = id;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    @Override
    public String toString() {
        final StringBuilder sb = new StringBuilder("Book{");
        sb.append("id=").append(id);
        sb.append(", name='").append(name).append('\'');
        sb.append('}');
        return sb.toString();
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Book book = (Book) o;
        return Objects.equals(id, book.id) &&
                Objects.equals(name, book.name);
    }

    @Override
    public int hashCode() {
        return Objects.hash(id, name);
    }
}

Is there a more elegant solution to this problem? Or is the use of deep cloning the only correct solution?

Answer 1

Changing state within a stream (as you are doing in peek) is an anti pattern. Don't do this.

I recommend something like this:

  public static List<Book> getListWithPrefixInName(List<Book> books) {
        return books.stream()
                .map(b -> Book.of(b.getId(), PREFIX + ": " + b.getName()))
                .collect(toList());
    }