Lombok: As a User I want to be able to use @CopyConstructor annotation to get rid of the boilerplate code

[Krasnyanskiy]

Suppose I have a class with a copy constructor:

@Data
class MyEntity {
    private String column;
    private String type;

    public MyEntity(MyEntity entity){
        this.column = entity.getColumn();
        this.type = entity.getType();
    }
}

It's nice to get rid of boilerplate code and replace it with annotation @CopyConstructor to let Lombok autogenerate a copy constructor for me.

@Data
@CopyConstructor
class MyEntity {
    private String column;
    private String type;
}

Reinier, what do you think about that?

_PS_

It's even better to provide an annotation with a value which describes the copying type:

@Data
@CopyConstructor(type = SHALLOW) // or DEEP
class MyEntity {
    private String column;
    private String type;
}

[luanpotter]

I think that is a very useful feature; would love seeing that :)
It would only take some concern as to whether it would shallow or deep, and how to control that.

[Krasnyanskiy]

@luanpotter Good point! It's always possible to specify a copy constructor type. I think it would be easy to implement such feature in Lombok. I found a few libs for cloning objects which can be useful: kryo, cloning, dozer.

And here is a possible API for copy constructor feature with respect to copy type:

@Data
@CopyConstructor(type = SHALLOW)
class MyEntity {
    private String column;
    private String type;
}

@Documented
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.SOURCE)
@interface CopyConstructor {
    CopyConstructorType type() default CopyConstructorType.DEEP;
}

enum CopyConstructorType {
    DEEP,
    SHALLOW
}

[smoyer64]

I'll also cast a vote for an annotation to create a copy constructor. In my typical use-cases, I'd say that I use a deep copy greater than 90% of the time, so that seems like a sensible default (to me).

[awhitford]

+1

This could help avoid some stupid hand-coding bugs.

[rspilker]

What about using @Builder(toBuilder = true)? In that case you can use

@Builder(toBuilder = true)
class Foo {
    // fields, etc
}

Foo foo = getReferenceToFooInstance();
Foo copy = foo.toBuilder().build();

Wouldn't that be enough?

[nyakto]

@rspilker Any benefit in builder here? @Data classes are mutable and can be considered as builders themselves (if copy constructor is available).

[nyakto]

@rspilker Sorry, didn't know there is already existing @Builder functionality )
Cool! It's a way to go!

[stephanos]

I assume the @Builder(toBuilder = true) trick only creates shallow copies of collections?

[rspilker]

Please stop posting +1 comments, and use the thumbs-up button under the original post.

[saboteurkid]

Hi guys,
Is there any plan for this feature?
@rspilker :+1: tks, I think toBuilder is an acceptable alternative.

[timjacobi]

Do you accept PRs? Would love to look at this.

[rspilker]

Before you start working on a pull request, we first need to figure out how to handle deep copies. Also, we need to find out why people would like to create a copy at all. In my experience, having Immutable objects is gaining more traction. For shallow copies we already have the toBuilder trick. What gap would adding CopyConstructor fill?

[Maaartinus]

@rspilker The deep copies thing is surely important, but note that it applies also to other constructors, setters and getters, where it wasn't considered to be a blocker. Here it's more important, but this feature could be started without deep copying, e.g., as @CopyConstructor(value=SHALLOW) with SHALLOW being the default and (for now) the only enum member.

For shallow copies we already have the toBuilder trick.

We do, but it's ugly and inefficient. It's very ugly as you only need copying for mutable objects, which usually need no Builder. It's inefficient as it does the copying twice.

[omega09]

Deep copying is very complicated and personally I don't think it's Lombok's job to do this.

The common technique is to serialize and deserialize using object streams because most object graphs are too complicated. You lose transient data this way. No reason for Lombok to do this for you, there are tools like Apache commons that can do it.

The other way is to traverse the object graph recursively and copy each object in the graph. It's like recursively annotating a shallow copy constructor. Here you'll have to take care of collections since they require to deep copy their elements. Since not all classes in the JDK permit this functionality, I don't think it'll be possible.

I agree with Maaartinus to implement a shallow copy constructor.

[omega09]

Please stop with the +1 comments. It notifies subscribers and creates noise. Use a thumbs up emoji on the top post instead.

[dveyarangi]

As an alternative, maybe @Builder or @CopyConstructure chaining is possible - make deep copy for member fields that are also annotated with @CopyConstructor

[wdroste]

As a matter of preference I like what Immutables does..

ImmutableValue.builder() .from(otherValue) // merges attribute value into builder .addBuz("b") .build();

[omega09]

@rspilker

What gap would adding CopyConstructor fill?

It could allow to instantiate a class from its superclass, providing a solution to problems like this.

[Maaartinus]

Deep copying is very complicated

@omega09
... and rather underspecified. You obviously mean the creation of a completely new object graph, which (except for immutable members) has nothing in common with the original. I guess, what's usually needed is a sort of depth-one copy: My object contains some mutable parts like collections (or some obsolete crap like Date), which it "owns". The members of these collections may be immutables or other objects, which are not "owned" and needn't be copied. Usually, you don't want to copy all members.

A typical example would be a shopping cart having a User containing a Multiset<Item>. When you need a copy of the cart, you only want to shallowly copy the Multiset, as the user and the items themselves are separate entities, which should stay as they're when you play with the cart.

I agree, that the full-depth copy constructor is outside of the Lombok scope, but I'd like to get a depth-one copy constructor one day. It's still a bit complicated without Lombok being omniscient w.r.t. to copying. However, it's possible to make it configurable:

@Getter @Setter
@CopyConstructor(value=DEPTH_ONE) // better name needed
class ShoppingCart {
    @NoCopier
    private User owner;

    // known to Lombok, copied automatically
    private final Set<Discount> discounts = new HashSet<>();

    @Copier(MyCopiers.Multiset) // user-defined class containing the copying method
    // @Copier(HashMultiset::create) // <- this would be much nicer
    private final Multiset<Item> items = HashMultiset.create();
}

The only argument of @Copier would be Function<T, T>. Now that even Android supports Java 8...

Alternatively, introduce @CopyingMethod working just like @ExtensionMethod does: providing a collection of methods to pick from.

[omega09]

For the second time, stop commenting with +1 and thumbs up the top comments instead. You're creating a lot of noise for nothing.

[omega09]

@Maaartinus

So delomboking the copy stuff, your class would be

@Getter @Setter
class ShoppingCart {

    public ShoppingCart(ShoppingCart cart) {
        ShoppingCart newCart = new ShoppingCart();
        newCart.setDiscounts(new HashSet<>(cart.getDiscounts());
        newCart.setItems(HashMultiset.create(cart.getItem());
    }

    private User owner;
    private final Set<Discount> discounts = new HashSet<>();
    private final Multiset<Item> items = HashMultiset.create();
}

?

[Maaartinus]

@omega09 Sort of... what you wrote won't compile, as discounts and items are final. (*) Additionally, you are creating a new instance of ShoppingCart in the constructor of ShoppingCart. And you forgot user. Nonetheless, I believe, you've got the idea right.

public ShoppingCart(ShoppingCart cart) {
    user = cart.user;

    // was initially empty, otherwise we'd need also discounts.clear()
    discounts.addAll(cart.discounts);

    // with a final field, the @Copier as specified won't work, but lets ignore it

    // VARIANT 1 (for a non-final field)
    // if lombok knew `HashMultiset` or if we could feed it with HashMultiset::create, then it'd use your line
    newCart.setItems(HashMultiset.create(cart.getItem());

    // VARIANT 2 (for a non-final field)
    // with @Copier(MyCopiers.Multiset) it would do
    newCart.setItems(MyCopiers.Multiset.apply(cart.multiset))
    // the user would most probably implement `apply` using `HashMultiset::create`
}

It turned more complicated than I thought, mostly because of my example not matching what I was thinking about.

Btw., some strange languages have assign, which copies the content of one instance into another. This allows to simulate the copy constructor easily and to reuse existing objects.

---

(*) Using final with mutable collections which are exposed to the outside makes sometimes sense as it allows to modify them without providing any methods for it. The disadvantage is that any user of the class may keep a copy and modify it at any time later, which leads to hard to find bugs. No idea why I did it.

[omega09]

@Maaartinus

what you wrote won't compile, as discounts and items are final

Yes, I completely ignored that :)

you are creating a new instance of ShoppingCart in the constructor of ShoppingCart

Oops, I changed the example a few times and it remained from a previous iteration where I was using a factory approach:

public static ShoppingCart copyDepthOne(ShoppingCart cart) {
    ShoppingCart newCart = new ShoppingCart(); // now makes sense
    // add/set everything
    return newCart;
}

which is more @Builder territory (and there are a few issues discussing this already).

And you forgot user.

But it's marked with @NoCopier so I though just don't copy. In your example I see that what you meant was a depth 0 (=shallow) copy. So to me it looks like the resolution is: if @NoCopier create a shallow copy, otherwise create a copy of depth as specified in @CopyConstructor(value) .

I would naively see in my mind something like this:

@Getter @Setter
@CopyConstructor
class ShoppingCart {

    // no annotation - no copy
    private Session session;

    @Copy(value = DEPTH_ZERO)
    private User owner;

    @Copy(value = DEPTH_ONE)
    private Collection<Discount> discounts = new CollectionImpl<>();

    @Copy(value = DEPTH_ONE, method = ForeignCollectionImpl::create) // let's assume this works somehow
    private ForeignCollection<Item> items = ForeignCollectionImpl.create();
}

which would create

ShoppingCart(ShoppingCart cart) {
    owner = cart.owner;

    // if we assume a copy constructor exists for the collection:
    discounts = new CollectionImpl<>(cart.getCollectionImpl())
    // or if we assume a setter/addAll exists for the collection's contents
    discounts = new CollectionImpl<>();
    discounts.addAll(cart.getDiscounts());

    // and now for the magic trick - is the given method a copy constructor or an empty constructor?
   items = ForeignCollectionImpl.create(cart.getItems());
}

BUT

• What if I annotate User owner with DEPTH_ONE? Generate owner = new User(cart.owner);? Doesn't
  this make sense mostly for collections? Is this worth it just for collections and fringe cases?
• Are the DEPTH_ONE classes supporting their own copy constructors? Are they supporting setters? I'm probably protected by compiler checks on the generated code but can Lombok check this without type resolution? I don't think so.

If this can be done then a full deep copy can be done by recursively applying depth one copy, which would solve the world's problems (Java core serialization anyone?). For DEPTH_ZERO I give my blessings and I think I can implement it myself even, except for maybe edge cases. For deeper copies which aren't just assignment expressions and involve invocations the details need to be really ironed out.

[Maaartinus]

@omega09

But it's marked with @NoCopier so I though just don't copy.

I see, it was a stupid name. Indeed, the object won't get copied at all, just a reference to it, so @ReferenceCopier might be better. For what you understood, maybe @CopyConstructor.Exclude could be a good name.

// no annotation - no copy

That's very error-prone. Moreover, it makes the @CopyConstructor copy nothing by default!

What if I annotate User owner with DEPTH_ONE? Generate owner = new User(cart.owner);?

I guess so. If there's such an annotation.

Doesn't this make sense mostly for collections?

Yes.

Are the DEPTH_ONE classes supporting their own copy constructors?

Lombok should handle classes it knows (JDK + Guava) perfectly. For others, it should have a simple default strategy like call their copy constructor and let the user handle it, if it doesn't compile. When the compile-time error is obvious, then it's fine. If it should lead to cryptic error, then giving up immediately with a sane message asking the user to provide their copier is better.

If this can be done then a full deep copy can be done by recursively applying depth one copy

I don't think so: You'd lose the object identity and any circle would lead to an infinite graph. IMHO the real full deep copy is grossly overrated, so far I had no single use for it.

the details need to be really ironed out.

Indeed. That's why I wrote that the shallow constructor should be done first and independently. Anything beyond that is orders of magnitude more complicated; even understanding what it should do is hard. IMHO it's impossible to do anything useful (beyond the shallow copy) without letting the user specify their own helper methods (and I see things like @Copy(value=DEPTH_ONE) just like shortcuts for the common cases).

Maybe we should write a copier using an ordinary annotation processor (which is much simpler) or look if someone already did. Or do it using reflection, which is even easier. Then we'd have a reference implementation to play with.

[omega09]

@Maaartinus

So let's decide on some annotation scheme. It looks to me like we're working with 3 options: exclude, shallow, and depth 1. Should the latter be the default for collections* while for all other types the default is shallow? Is the level specified in the class annotation and then overriden in the field annotations?

*Don't think we can check is a field is a collection in Lombok, while we can with reflection.

[Maaartinus]

@omega09
IIUYC shallow on a field means copy the reference but not the object. That's confusing as the "shallowness" for the whole object means "reference" for the fields. Similarly for depth 1. I'd call it differently:

• @CopyConstructor.Exclude: leave it as is, probably null
• @CopyConstructor.Reference: copy the reference to the old object
• @CopyConstructor.Copy:
  
  
  
  • for known immutables (String, Guava immutables like ImmutableSet, ...), copy the reference
  
  
  • for other known collections and maps, create a new one as a shallow copy (i.e., referring to the same objects as the old one)
  
  
  • for other known classes, do what's needed (Date.clone or alike)
  
  
  • for anything else, invoke their copy constructor, which may or may not compile
  
  
• @CopyConstructor.Copy(MyCopyingHelper.class): invoke MyCopyingHelper.copy(thisField), which may or may not compile, just like @ExtensionMethod

On the whole class, I see three possibilities:

• @CopyConstructor: a shallow copy, using @CopyConstructor.Reference on fields by default
• @CopyConstructor(DEPTH_1): a depth 1 copy, using @CopyConstructor.Copy on fields by default
• @CopyConstructor(MyCopyingHelper.class): a user-defined constructor, using @CopyConstructor.Copy(MyCopyingHelper.class) on fields by default

Don't think we can check is a field is a collection in Lombok

Agreed. I'd only check for known classes according to their declared type.

[omega09]

@Maaartinus

Alright, I'll fork something this weekend.

[omega09]

@Maaartinus

I created a fork. Summary of current stage:

New annotations

• @CopyConstructor valid on classes only. Has a Depth property which is an enum with 3 entries EXCLUDE, REFERENCE and ONE. The default Depth is REFERENCE.
• @CopyConstructor.Exclude, @CopyConstructor.Reference and @CopyConstructor.Copy valid on fields only. @Copy has a Class<?> property specifying the copy helper class.

Currently only Exclude and Reference are supported.

Usage

• All fields are treated with the Depth property of the class annotation unless they specify their own.
• If more than one field annotation is applied it is a compilation error.
• If a field annotation is applied to a static field it is a compilation error.
• If a field annotation is applied to a field in a class without the class annotation it is a compilation warning for the annotation being meaningless.
• If Exclude is applied to a final field initialized to null it is a compilation warning for not assigning a value in the constructor. (Should probably be changed to issue the warning for a constructor with depth = EXCLUDE unless the explicit field annotation is used.)

Implementation

• The generated constructor is public with an argument of the type of the annotated type (duh). Possibly need to allow access level selection.
• All fields which are invalid (e.g., static) or which resolved to EXCLUDE (implicitly or explicitly) are ignored.
• All fields which resolved to REFERENCE are given an assignment from the getter of the constructor argument. If a REFERENCE copy field does not have a getter a compilation error will be given. Possibly need to allow useGetters = false.

Currently only works with ejc (Eclipse). There's also room for some cleanup and corrections after adding Copy support.

Copy behavior

I'm not sure how to treat "known" types. Collections can use addAll which can be used, but how would you know for other types what to do? Check for implementation of clone?

Example

The following code

@CopyConstructor(depth = Depth.EXCLUDE)
@Getter
class Test {

    static int i;

    @Reference
    boolean b;

    @Reference
    List<String> list = new ArrayList<>();

    @Exclude
    Object o;
}

after delomboking the CopyConstructor annotations is

@Getter
class Test {
    static int i;
    boolean b;
    List<String> list = new ArrayList<>();
    Object o;

    @java.lang.SuppressWarnings("all")
    @javax.annotation.Generated("lombok")
    public Test(final Test test) {
        super();
        this.b = test.isB();
        this.list = test.getList();
    }
}

[balajeetm]

Why is this still open? It's very useful feature indeed.

[omega09]

@balajeetm You mean why it's not implemented? It needs to be finished and then a PR need to get reviewed. I more or less forgot about it since I posted the initial implementation. The previous post's "Copy behavior" section contains some questions that need to be answered.

[archenroot]

@omega09 - Can you make summary of questions open regarding this cool feature?

[janrieke]

What about a parameter to control whether to call the @CopyConstructor of the superclass (instead of calling super())? This would allow for copying fields from superclasses.

[FeanorsCurse]

I just tried the Builder and it's not working properly, see also: https://github.com/peichhorn/lombok-pg/issues/78 - it is not considering inherited attributes, which means it's basically unusable for me (want a replacement for clone()). So I would also prefer a dedicated @CopyConstructor, along with option to set visibility to private.

[rzwitserloot]

Making a deep clone is not possible: There is no general way to clone objects; you need to know what you're dealing with. Integer, String, etc are immutable and a deep clone should just copy the reference, though sometimes for exotic reasons you really do want to deep clone an immutable in which case you need custom code.

arrays and such need special treatment to clone. You can't just go around and call clone() on things; clone() generally makes shallow clones, and once you commit to making a deep clone, that should be turtles all the way down.

Thus, the only feasible thing we can add is a shallow clone, and toBuilder already suffices for that use case.

We will not add this; please don't waste your time with PRs or further discussion.

[antoniomacri]

toBuilder is not usable if there is a hierarchy of objects.

Am I missing something?

[randakar]

Yes.

Of course you can use toBuilder with a hierarchy, just make sure everything
in the hierarchy has builders / toBuilder and you can do nested builder
calls:

Given a toplevel object with an List as as inner field, which contains leaf
nodes created by a method named createLeaf():

TopLevel newtopLevel = topLevel.toBuilder()
.innerField(topLevel.getInnerField().stream()
.map(t -> t.toBuilder()
.leaf(createLeaf(t))
.build())
.collect(Collectors.toList()))
.build();

The only time that you get in trouble with this pattern if there are
backwards references to the top level object, and even that has solutions.
(if not terribly pretty ones, but no uglier than if you do this the
conventional way.)

On Mon, Oct 29, 2018 at 2:47 PM Antonio Macrì [e-mail]
wrote:

toBuilder is not usable if there is a hierarchy of objects.

Am I missing something?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/908#issuecomment-433915647,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKCRTxwd3HARbpkcs3LX7d0-pl1YGo9ks5upwbggaJpZM4F1ZX-
.

--
"Don't only practice your art, but force your way into it's secrets, for it
and knowledge can raise men to the divine."
-- Ludwig von Beethoven

[janrieke]

And @SuperBuilder will get support for toBuilder with the next release.

[antoniomacri]

This is the exact case I have:

class Super {
    private int superVars;

    public Super(Super other) {
        // copy constructor
        this.superVars = other.superVars;
    }
}

class Sub extends Super {
    private String subVars;

    public Sub(Super s) {  // <-- Notice Super, not Sub
        super(s);
        this.subVars = "something else";
    }
}

toBuilder cannot be used as of now.

Will SuperBuilder be suitable for this case?

[janrieke]

Yes. You may have to customize it, but it should be possible.