search

lishunli / projectlombok

Automatically exported from code.google.com/p/projectlombok
0 stars 0 forks source link

Add a @Builder annotation for the builder pattern. #16

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Useful features it should have:

 - integrate with final parameters (e.g: Know which parameters are initialized via the constructor, 
and which via setX methods).
 - allow extending the builder with custom methods somehow.
 - Tight integration with @Data - e.g. allow making the constructor generated by @Data to be 
private, forcing all construction to occur via builder. Also sort out 
constructor vs. setter method 
initialization automatically.
 - Set up a static method to create a builder instead of forcing new FooBuilder(). Consider allowing 
certain variables to be passed directly to the create() method (i.e. mandatory 
parameters, especially 
if there's only one or two).

Original issue reported on code.google.com by reini...@gmail.com on 5 Aug 2009 at 2:13

GoogleCodeExporter commented 9 years ago 
Issue 15 has been merged into this issue.

Original comment by reini...@gmail.com on 5 Aug 2009 at 2:14

GoogleCodeExporter commented 9 years ago 
This is an interesting take on the builder pattern, definitely something we 
should look into when we make 
@Builder for lombok. It generates an absolute explosion of boilerplate, but, 
that's what lombok's for, after all!

http://groups.google.com/group/javaposse/browse_thread/thread/98842b3a13ac61da

Original comment by reini...@gmail.com on 23 Nov 2009 at 8:39

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
Would be very nice to have this feature - I've just written a ton of boiler 
plate that made me consider simply using the bean style instead.  Perhaps a 
very simple implementation could act as a stop-gap until the more difficult 
features are figured out.

Basically, I needed something that acts exactly like @Setter but generates

public MyBuilder setting(Thing setting)
{
  this.setting= setting;
  return this;
}

Original comment by jdpatterson on 20 Jun 2011 at 6:17

GoogleCodeExporter commented 9 years ago 
There is an extension that creates builders you can find it here 
https://github.com/peichhorn/lombok-pg

Original comment by framiere on 7 Sep 2011 at 1:21

GoogleCodeExporter commented 9 years ago 
Would be a great feature to have. Along the lines of Effective Java 2 I'd love 
to have an @Builder annotation that would result in something like this: 
https://gist.github.com/1281709

@Builder would add a copy constructor to the bean, clone all of the bean's 
setters (except they return the builder) and build() would result in a new 
object on each call.

Original comment by eric.dalquist on 12 Oct 2011 at 4:29

GoogleCodeExporter commented 9 years ago 
Support for the Builder pattern mentioned by eric.dalquist would be great but I 
prefer setters not named "setFoo(int)" but "foo(int)" in the builder (also 
mentioned that way in Effective Java 2)

Original comment by peter.fi...@gmail.com on 28 Feb 2012 at 2:50

GoogleCodeExporter commented 9 years ago 
This is a killer feature for those of us that rely on builders to provide some 
mutable behavior on non-trivial, immutable code bases. Is there any update on 
this work?

Original comment by torr0...@gmail.com on 12 Mar 2012 at 6:55

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
FWIW, I usually end up making an abstract builder and a concrete builder:

abstract public class BaseSomeStringBuilder<T> {
  @Getter
  private String prefix = "";

  abstract protected T me ();

  public T setPrefix (final String prefix) {
    this.prefix = prefix;
    return me();
  }

  ...
}

public class SpecificListBuilder extends 
BaseSomeStringBuilder<SpecificListBuilder> {

  public SpecificListBuilder () {
    super();
    setPrefix("{");
    ...
  }

  @Override
  protected SpecificListBuilder me () {
    return this;
  }

  ...
}

Original comment by joew...@gmail.com on 12 Mar 2012 at 11:18

GoogleCodeExporter commented 9 years ago 
Are there any plans to implement this feature? It would be a really useful 
thing to have.

Original comment by wojtek.g...@gmail.com on 8 Feb 2013 at 10:42

GoogleCodeExporter commented 9 years ago 
I would be happy with just simple builder style setter as jdpatterson suggested 
in #4. It shouldn't be a lot of work.

Original comment by PavelCib...@gmail.com on 15 Feb 2013 at 4:33

GoogleCodeExporter commented 9 years ago 
There is already @FluentSetter annotation in lombok-pg (fork of lombok). Can 
you please merge this into lombok.

Original comment by PavelCib...@gmail.com on 19 Feb 2013 at 10:56

GoogleCodeExporter commented 9 years ago 
Currently there is an experimental feature in lombok itself, which allows 
fluent setters:  http://projectlombok.org/features/experimental/Accessors.html

Original comment by askon...@gmail.com on 19 Feb 2013 at 3:59

GoogleCodeExporter commented 9 years ago 
>Currently there is an experimental feature in lombok itself, which allows 
fluent setters:  http://projectlombok.org/features/experimental/Accessors.html

Interesting! I would strongly consider that the choice of getter/setter type 
not just be an either/or, however. There are reasons to provide bean properties 
(primarily for reflection) and they are different from the convenience that a 
fluent getter/setter provides. I would want the ability to have both, if I so 
choose.

(Personally, I wouldn't bother using fluent getters, since it just saves me 3 
characters, but a fluent setter is a big advantage.)

Original comment by jmsa...@gmail.com on 19 Feb 2013 at 4:21

GoogleCodeExporter commented 9 years ago 
On the documentation page regarding the Accessors annotation, it states:
"Current status: positive - Currently we feel this feature may move out of 
experimental status with no or minor changes soon."

When do you plan to move it out of experimental status?

Original comment by wojtek.g...@gmail.com on 19 Feb 2013 at 6:38

GoogleCodeExporter commented 9 years ago 
To be honest, I don't thing that @Accessors is an option for Builder pattern.

Example:
public static class Builder {
    @Getter @Setter @FluentSetter
    private int id = 10;
}

Result:
 public static class Builder {
    private int id = 10;

    public int getId() {
        return id;
    }
    public void setId(int id) {
        this.id = id;
    }
    public Builder id(int id) {
        this.id = id;
        return this;
    }
}

It is just three annotations and I will get getter and two setters. I need all 
of this, to be able use Builder pattern + Bean validation (for forms). How can 
I do this with @Accessors? Fluent getter? Guys, I think that you overcomplicate 
simple thing.

Original comment by PavelCib...@gmail.com on 22 Feb 2013 at 12:04

GoogleCodeExporter commented 9 years ago 
You want a class that is Bean *and* Builder?

I currently use a couple of builders like this:

@Data
@AllArgsConstructor(access = AccessLevel.PRIVATE)
public class Foo {
    private final int id;
    private final String name;

    public static Builder builder() {
        return new Builder();
    }

    @Accessors(fluent = true)
    @NoArgsConstructor
    public static final class Builder {

        private int id = 0;
        private String name = "";

        public Foo build() {
            return new Foo(id, name);
        }
    }
}

Gives me the power of a builder pattern and a nice clean POJO.

Original comment by askon...@gmail.com on 26 Feb 2013 at 9:50

GoogleCodeExporter commented 9 years ago 
The previous post also gives some idea of how a @Builder annotation could be 
implemented that works for simple builders. 
Effectively, the pojo with builder above displays a lot of boiler plate: the 
fields for the builder and for the pojo are the same, the static method 
builder() is predictable and also the build() method of the Builder class 
itself is predictable. The only problem you might have is with non-standard 
default values for the fields of the builder, if the fields are final (i.e. the 
name = "", instead of name = null).

A resulting class with @Builder would like like:

@Builder
public class Foo {
    private int id;
    private String name;
}

Note: The @Builder could include the @Data and @AllArgsConstructor(access = 
AccessLevel.PRIVATE) by default.
Note 2: I've removed the final keywords on the fields to avoid the problem with 
non-standard default values for now. This is something that requires more ideas.

Original comment by c.m.kui...@gmail.com on 28 Feb 2013 at 8:59

GoogleCodeExporter commented 9 years ago 
Hi,

I would realy love to see the @Builder Annotation in Lombok (like in lombok-pg) 
but instead of

Person.person()...build();

use

Person.create()...build();

I think it is easier to understand for people who read my code...

Original comment by kale.sa...@gmail.com on 30 Mar 2013 at 11:59

GoogleCodeExporter commented 9 years ago 
This would be lovely, please do this.  Also, please add an unbuild method? This 
would create a builder based on the object in question.
Thus if object personName1 has firstName = "John" and lastName = "Smith" you 
could do this:

personName2 = personName1.unbuild().lastName("Johnson").build();

yielding personName2 with firstName = "John" and lastName = "Johnson"

Also you could implement clone() pretty directly from this if you wanted.

Original comment by danielma...@gmail.com on 2 May 2013 at 3:22

GoogleCodeExporter commented 9 years ago 
re: comment #20:

>I would realy love to see the @Builder Annotation in Lombok (like in lombok-pg)
>but instead of
>
>Person.person()...build();
>
>use
>
>Person.create()...build();
>
>I think it is easier to understand for people who read my code...

Person.builder()...build() would be better. You're creating a builder, not a 
Person object, so Person.create() is misleading -- as is Person.persion(), that 
I agree with.

re: comment #21:

>Also, please add an unbuild method? This would create a builder based on the 
object in question.

Please do NOT call it "unbuild", that carries a connotation of destroying an 
object. This general idea is very useful, but more appropriate and 
straightforward method would be to overload the method that creates the builder 
and allow passing an existing argument to initialize the builder; or add a new 
method, e.g.:

   // create from scratch
   Person person1 = Person.builder().firstName("Adam")...build();

   // create and initialize builder with an existing prototype
   Person person2 = Person.builder(person1).firstName("Eve").build();
   Person person2 = Person.builderBasedOn(person1).firstName("Eve").build();
   Person person2 = Person.basedOn(person1).firstName("Eve").build();

"builderBasedOn" is accurate but overly verbose, and I think "basedOn" isn't
obvious enough that it would create a builder and not an actual Person object.

If you did want to create an instance method of doing the same thing, please 
take
great care in choosing a method name, which has a good match to its function.
I can't think of a great verb for it; the best I can come up with is "override"
or "redesign", and if you don't need a verb then "builder" might be ok.

   Person person2 = person1.override().firstName("Eve").build();
   Person person2 = person1.redesign().firstName("Eve").build();
   Person person2 = person1.builder().firstName("Eve").build();

And I'm not sure I agree that the word "override" is close enough to what it 
does =
create a builder initialized using the object in question. The words "redesign" 
and "builder" are closer... but they seem a little ambiguous to me.

Original comment by jmsa...@gmail.com on 2 May 2013 at 4:44

GoogleCodeExporter commented 9 years ago 
[continued]

...the only reason I can think of, where an instance method might be more 
appropriate than a static class method, to create a new builder that is 
initialized with the object in question, is *subclassing*.

Person bruceJenner = 
Person.builder().firstName("Bruce").lastName("Jenner").build();
Person batman = 
SuperPerson.builder().firstName("Bruce").lastName("Wayne").secretIdentity("Batma
n").build();
Person steve = Person.builder(batman).firstName("Steve").build(); // creates a 
normal Person named Steve Wayne
Person gnatman = 
batman.redesign().firstName("Wayne").secretIdentity("Gnatman").build();
// creates a SuperPerson Wayne Wayne aka Gnatman, because Person.redesign() is 
overridden to create the right kind of Builder object.
// Downside: if all you have is a Person object to start with, how are you 
supposed
// to know that the builder object is really a SuperPerson.Builder and has a 
"secretIdentity" method?

Original comment by jmsa...@gmail.com on 2 May 2013 at 4:55

GoogleCodeExporter commented 9 years ago 
+1 this is the most starred issue in the bug tracker

IIRC, Google's protobuf Java library generates it's classes with a .builder() 
method that will create an editable builder from an immutable class. It's a 
really great pattern that's a joy to work with.

Original comment by benjamin...@gmail.com on 3 May 2013 at 5:26

GoogleCodeExporter commented 9 years ago 
re: comments 22, 23, 24:
I like the Person.builder() name.  "unbuild" was a false analogy, given that 
it's on the Person, not the Builder.  I'm in.  I just want it please.

Original comment by danielma...@gmail.com on 7 May 2013 at 5:48

GoogleCodeExporter commented 9 years ago 
We think we have a design ready for implementation for @Builder. It's 
documented here:

https://groups.google.com/forum/?fromgroups#!topic/project-lombok/Hw9Nvku-SNY

We'll start work on this within a few days, so, please post your comments on 
the API now :)

Original comment by reini...@gmail.com on 16 May 2013 at 9:41

GoogleCodeExporter commented 9 years ago 
.... and, it's done!

I just pushed out an edge release with this feature included. It'll stay in 
experimental for only a few months while we gather final feedback, our 
intention is to move this to the core package ASAP.

Give it a shot: http://projectlombok.org/features/experimental/Builder.html

Original comment by reini...@gmail.com on 18 Jun 2013 at 2:29

GoogleCodeExporter commented 9 years ago 
Good news! Love it! but there is a big bug:

With @Builder you can't invoke default constructor anymore.

new SomeObject() -> doensn't work.
One can use SomeObject.builder().build() as a workarround but this will not 
work with libraries which expect a default constructor like mybatis.

Original comment by kale.sa...@gmail.com on 26 Jul 2013 at 12:07

GoogleCodeExporter commented 9 years ago 
kale.samil:

Just write this:

@Builder @AllArgsConstructor(access=AccessLevel.PRIVATE) @NoArgsConstructor
public class Thingie {}

and you'll have both new SomeObject() and SomeObject.builder().

The why:

* @Builder really only makes sense on a method / constructor. Putting @Builder 
on class is shorthand for 'make a constructor for every argument, and imagine 
this annotation was on that constructor'.

* Unfortunately, if you add an explicit constructor, either by actually writing 
one out or by using one of lombok's @XArgsConstructor annotations, @Builder 
will instead assume that constructor will evidently be the appropriate one and 
will no longer generate one itself.

* So, you force lombok to generate both of them, making the all-args 
constructor private because in general it's a good idea to force people to do 
the same thing in the same way every time (so, use builder, don't give them a 
choice between either using the builder or using the all-args constructor).

Original comment by reini...@gmail.com on 26 Jul 2013 at 3:01

GoogleCodeExporter commented 9 years ago 
I just tried it out, great job! I especially like the idea of supporting 
Builder on static methods in general rather than just classes.

There is one critical thing that I don't like however, and I will not use this 
feature unless it is fixed : it is currently possible to forget some of the 
arguments before the build() method is called. This is really dangerous because 
adding new parameters to functions that provide a builder will NOT cause 
compile time errors on client calls that do not add the new arguments.

I am currently using lombok-pg uniquely because of its implementation of the 
Builder pattern, that addresses this exact problem. lombok-pg has other 
problems though, and is not really supported anymore, which is why I would love 
to see the Builder pattern included in standard lombok.

I guess you know how to implement a "safe" builder, but just in case, here is 
how it works: In addition to a Builder class, a series of interfaces is 
generated, each allowing the setting of a single parameter, and returning the 
interface that allows the setting of the next one. Finally, the last interface 
is the only one that allows to call the build() method. the Builder class is 
private, and implements all of the interfaces, and the static method returns 
the first interface in the chain effectively a new instance of the Builder 
class.

In the lombok-pg implementation, there are tricky issues with optional class 
parameters, but this problem will not exist here thanks to the smart choice of 
supporting Builders on static methods and constructors, and not classes.

Have you put some thought on this feature yet? Is there a good reason not to 
implement it? I can imagine that it is much harder to implement, but for me it 
really is a killer issue. Furthermore, it will not be backward compatible if 
implemented later because it enforces argument ordering.

Original comment by mofle...@gmail.com on 2 Aug 2013 at 8:07

GoogleCodeExporter commented 9 years ago 
This is coming along ok. However, there are some major shortcomings when 
compared to lombok-pg's implementation. The following are blockers for my 
company:

Build.Extension - Need to be able to add custom methods to handle fine grain 
api crafting

Builder constructor arg - Need to ensure the validity of the state of an object 
according to specific rules upon instantiation.

Builder method prefix - We have a standard that requires us to prefix methods 
with the string "with". This doesn't appear possible in lombok's builder.

Respect Accessors - We have an internal field naming convention that requires a 
prefix on fields. With Getter, we use Accessor(prefix) to trim it off. With 
builder it is still exposed.

Are these features intended to be rolled into lombok's implementation at any 
point?

Original comment by torr0...@gmail.com on 2 Aug 2013 at 3:34

GoogleCodeExporter commented 9 years ago 
* Build.Extension: Just create the builder class and stick your extension 
methods in there, voila.

constructor arg: put the @Builder annotation on a static method (which can be 
private if you prefer) and do the checking there - or just do that in the 
constructor, and put @Builder on said constructor. Which can also be private if 
you prefer.

with: withX implies (by convention) that clones are being made. You should 
change your standard; we won't cater to it.

respect accessors: We do respect accessors, to a point; while we look at 
accessors to determine which prefix, if any, we should chop off field names, we 
don't look at it when making fields ourselves. You should probably adhere a 
little less strictly to these rules of thumb; the only time you'd even refer to 
those fields is if you're sticking extensions into the builder, that's 
literally it. You'd never even see the otherwise.

Original comment by reini...@gmail.com on 3 Aug 2013 at 3:27

GoogleCodeExporter commented 9 years ago 
That's mostly good news. 

In regards to the "with" question, I do not believe this implies clones. I am 
not familiar with this being the case. In fact, I cannot recall a case in the 
projects I have come in contact with where this is the truth. Perhaps this is a 
philosophical "pattern vs practice" situation. At any rate, I do not think that 
it would be too onerous on lombok to provide the ability to prefix the fluent 
setter names with a configurable string.

Standards are not only applicable to internal developers. They communicate a 
clear and consistent view of our API to all internal developers and clients. 
While I do not expect the lombok team to care greatly about our situation, it 
seems as if this small change - that was included in lombok-pg for good reason 
- would make the already terrific lombok library a must have. The Builder 
pattern is so tedious to implement that this would be a truly killer feature if 
it met the general case. While this request may not be needed or desired by 
literally 100% of lombok users, based on many builders I've seen in the wild, 
it is very common to prefix the setter methods. This would help lombok achieve 
additional use case coverage.

On the Accessors bit, I must have missed this. 

Thanks for a terrific lib!

Original comment by torr0...@gmail.com on 3 Aug 2013 at 6:21

GoogleCodeExporter commented 9 years ago 
Just as a quick case in point, Bob the Builder 
(https://code.google.com/a/eclipselabs.org/p/bob-the-builder/) -a popular 
builder plugin for eclipse - simply returns this from its "with" methods. I 
have only ever seen this approach. Not saying cloning on with doesn't exist, 
but I've been around for a while now and it is anecdotally true that returning 
this is the common case.

Original comment by torr0...@gmail.com on 3 Aug 2013 at 6:24

GoogleCodeExporter commented 9 years ago 
Actually, testing reveals that the builder does not chop off Accessor prefixes:

public class FooTest
{

    @Test
    public void test()
    {
        Foo.builder().test("Test").theTitle("something else").build();

        new Foo.FooBuilder();
    }

    @Accessors(prefix = "the")
    @Builder(builderClassName = "FooBuilder")
    @Getter
    @ToString
    public static class Foo
    {
        private String theTitle;

        private static class FooBuilder
        {
            private FooBuilder test(String aSomething)
            {
                System.out.println(aSomething);
                return this;
            }
        }
    }
}

Also, the builder's visibility appears to be package level. Is this required?

Original comment by torr0...@gmail.com on 3 Aug 2013 at 8:56

GoogleCodeExporter commented 9 years ago 
Great job on the @Builder.

There is one feature I miss: The ability to exclude fields from the builder. 
For instance my class contains a List. I added an extension method withX() to 
fill the list item-per-item. The generated x(List) is now obsolete, but cannot 
be removed.

Original comment by middelk...@gmail.com on 5 Aug 2013 at 12:23

GoogleCodeExporter commented 9 years ago 
Hi,

I posted some time ago a comment about builder safety (comment #30), but have 
not had any feedback yet.

Does anyone have any input on this topic?

Original comment by mofle...@gmail.com on 28 Aug 2013 at 6:51

GoogleCodeExporter commented 9 years ago 
RE Comment 30 (request for pg-style cascading interfaces).

This is not going to happen; it turns into a mess when you have optional 
parameters, it's definitely not a common sight (based on the notion that it 
takes tens of pages if done 'by hand'), it is virtually not delomokable (again, 
tons of pages), and flooding the class space is still not something that the 
JVM is really good at (see for example how closures are now done with 
methodhandles and synthetic methods instead of inner classes).

You can add a runtime check quite easily though: Annotate a static method or 
constructor instead of a class with @Builder, and check for the nullness of the 
new parameters, throwing an exception. If null is actually a valid choice but 
you want them to make it explicitly, you can initialize the field in the 
builder class with a sentinel value that indicates: This wasn't set at all.

Original comment by reini...@gmail.com on 28 Aug 2013 at 8:06

GoogleCodeExporter commented 9 years ago 
RE comment 36 (field exclusion): There are 2 work-arounds: Create a constructor 
/ static method without the list and @Builder that. But now you have no place 
to send the list, so that won't really work for this admittedly common use 
case. The second is to create your own x() method but make it private, 
effectively removing it.

Hmm, maybe an exclude is worth doing, though the naming has to be very precise 
because 'exclude' can mean so many things. In particular, this meaning seems 
like the more logical one:

> Don't make the method in the builder. Don't make the field. Don't pass it. 
Exclude it from the generated all-args constructor.

Whereas the useful meaning is:

> Just skip the method; still make the field, still pass it, do not exclude it 
from the generated all-args constructor.

I'd have to be called 'skipMethod' or some such.

Original comment by reini...@gmail.com on 28 Aug 2013 at 8:09

GoogleCodeExporter commented 9 years ago 
RE Comment 38

Too bad... I expected the argument of class space flooding, but was not sure 
how critical that would be.

As for the rest of them...

- there are no 'optional parameters' to static methods as far as I know (again, 
smart choice to support them with the builder pattern)
- not delombokable... hm okay, one could argue that the builder pattern 
currently supported is hardly delombokable as well...
- runtime check : anything that can be done at compile time should in my 
opinion done there. It is such a huge gain to avoid having to unit test all 
these calls! Plus, I used the weaker pattern in several projects quite heavily, 
and still regret that the safer pattern was not used right away.

Anyway, I know now that I can quit hoping to have safe builders in mainstream 
lombok, thank you for this answer.

Original comment by mofle...@gmail.com on 28 Aug 2013 at 2:25

GoogleCodeExporter commented 9 years ago 
RE Comment 38

Hi!

This is great news you consider adding builder pattern to lombok!

Regarding your point on strongly typed builder pattern. 

The builder pattern has been proposed as a way to mitigate the missing java 
functionnality: named constructor arguments.
Personnaly I will typically use this pattern when I'm a service provider/API 
and I want to be sure that client can construct easily bean to pass to my 
service. I won't use it when I have full control over the code.

The solution you propose, in the case of an client error, is to notify her that 
she was not able to pass to my service a valid object.
The question that naturally follows, is: Why can her build an invalid object at 
the first place ?
I'm not that fan about the exception based workflow, when I would have been 
able to enforce correct behavior first :/

Correct me if I'm wrong, but as far as I understand, lombok mainly goal consist 
of boilerplate busting.

Writing all the assertions to check if the arguments were passed, looks like to 
be an interresting boilerplate too :D

Finally, I quote you "it takes tens of pages if done 'by hand'". 
We both agree that this point is a great success for a boiler plate buster 
right ?

Original comment by david.jo...@gmail.com on 6 Sep 2013 at 3:09

GoogleCodeExporter commented 9 years ago 
I sent this already per email (whatever happened to it), but it belongs here. 
So sorry for possibly double posting.

To commet #38: The sentinel value is not always applicable, especially for 
primitives and nullable enums. Even if it's applicable, it's rather hacky.

There's something, Lombok could do without much effort.

1. Generate the runtime check. You only need a bitmap, actually a single long 
must suffice.

2. Do a compile-time check in simple cases like this

@Builder(required=true) @AllArgsConstructor
class X {
  int a, b, c;
}

X wrong(int x) {
  // generate a compile-time error
  return X.builder().a(x).b(x+1).build();
}

The syntax could be like follows:
- @Builder(required=true) making all arguments required (target: TYPE, METHOD, 
CONSTRUCTOR)
- @Builder.Required making a single argument required and 
@Builder.Required(false) doing the opposite (target: PARAMETER, FIELD)

I'm not advocating this, just pointing to the possibilities.

Original comment by Maaarti...@gmail.com on 7 Sep 2013 at 11:49