-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a @Builder annotation for the builder pattern. #89
Comments
👤 reinierz 🕗 Aug 05, 2009 at 14:13 UTC Useful features it should have:
|
👤 reinierz 🕗 Aug 05, 2009 at 14:14 UTC Issue #88 has been merged into this issue. |
👤 reinierz 🕗 Nov 23, 2009 at 20:39 UTC This is an interesting take on the builder pattern, definitely something we should look into when we make http://groups.google.com/group/javaposse/browse_thread/thread/98842b3a13ac61da |
👤 jdpatterson 🕗 Jun 20, 2011 at 06:17 UTC 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) |
👤 framiere 🕗 Sep 07, 2011 at 13:21 UTC There is an extension that creates builders you can find it here https://github.com/peichhorn/lombok-pg |
👤 eric.dalquist 🕗 Oct 12, 2011 at 16:29 UTC 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. |
👤 peter.fichtner 🕗 Feb 28, 2012 at 14:50 UTC 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) |
👤 torr0101 🕗 Mar 12, 2012 at 18:55 UTC 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? |
👤 joewalp 🕗 Mar 12, 2012 at 23:18 UTC FWIW, I usually end up making an abstract builder and a concrete builder: abstract public class BaseSomeStringBuilder { abstract protected T me (); public T setPrefix (final String prefix) { ... public class SpecificListBuilder extends BaseSomeStringBuilder { public SpecificListBuilder () { @ Override ... |
👤 wojtek.gorski 🕗 Feb 08, 2013 at 10:42 UTC Are there any plans to implement this feature? It would be a really useful thing to have. |
👤 PavelCibulka 🕗 Feb 15, 2013 at 16:33 UTC I would be happy with just simple builder style setter as jdpatterson suggested in ﹟4. It shouldn't be a lot of work. |
👤 PavelCibulka 🕗 Feb 19, 2013 at 10:56 UTC There is already @ FluentSetter annotation in lombok-pg (fork of lombok). Can you please merge this into lombok. |
👤 askoning 🕗 Feb 19, 2013 at 15:59 UTC Currently there is an experimental feature in lombok itself, which allows fluent setters: https://projectlombok.org/features/experimental/Accessors.html |
👤 jmsachs 🕗 Feb 19, 2013 at 16:21 UTC
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.) |
👤 wojtek.gorski 🕗 Feb 19, 2013 at 18:38 UTC On the documentation page regarding the Accessors annotation, it states: When do you plan to move it out of experimental status? |
👤 PavelCibulka 🕗 Feb 22, 2013 at 12:04 UTC To be honest, I don't thing that @ Accessors is an option for Builder pattern. Example: Result:
} 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. |
👤 askoning 🕗 Feb 26, 2013 at 09:50 UTC You want a class that is Bean and Builder? I currently use a couple of builders like this: @ Data
} Gives me the power of a builder pattern and a nice clean POJO. |
👤 c.m.kuiper 🕗 Feb 28, 2013 at 08:59 UTC The previous post also gives some idea of how a @ Builder annotation could be implemented that works for simple builders. A resulting class with @ Builder would like like: @ Builder Note: The @ Builder could include the @ Data and @ AllArgsConstructor(access = AccessLevel.PRIVATE) by default. |
👤 kale.samil 🕗 Mar 30, 2013 at 11:59 UTC 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... |
👤 danielmalcolmabraham 🕗 May 02, 2013 at 15:22 UTC This would be lovely, please do this. Also, please add an unbuild method? This would create a builder based on the object in question. 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. |
👤 jmsachs 🕗 May 02, 2013 at 16:44 UTC re: comment ﹟20:
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:
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 // create and initialize builder with an existing prototype "builderBasedOn" is accurate but overly verbose, and I think "basedOn" isn't If you did want to create an instance method of doing the same thing, please take Person person2 = person1.override().firstName("Eve").build(); And I'm not sure I agree that the word "override" is close enough to what it does = |
👤 jmsachs 🕗 May 02, 2013 at 16:55 UTC [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(); |
👤 benjamin.j.mccann 🕗 May 03, 2013 at 05:26 UTC +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. |
👤 danielmalcolmabraham 🕗 May 07, 2013 at 17:48 UTC re: comments 22, 23, 24: |
👤 reinierz 🕗 May 16, 2013 at 21:41 UTC 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 :) |
👤 reinierz 🕗 Jun 18, 2013 at 02:29 UTC .... 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: https://projectlombok.org/features/experimental/Builder.html |
👤 kale.samil 🕗 Jul 26, 2013 at 12:07 UTC Good news! Love it! but there is a big bug: With @ Builder you can't invoke default constructor anymore. new SomeObject() -> doensn't work. |
👤 reinierz 🕗 Jul 26, 2013 at 15:01 UTC kale.samil: Just write this: @ Builder @ AllArgsConstructor(access=AccessLevel.PRIVATE) @ NoArgsConstructor and you'll have both new SomeObject() and SomeObject.builder(). The why:
|
👤 mofleury 🕗 Aug 02, 2013 at 08:07 UTC 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. |
👤 torr0101 🕗 Aug 02, 2013 at 15:34 UTC 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? |
👤 reinierz 🕗 Aug 03, 2013 at 03:27 UTC
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. |
👤 torr0101 🕗 Aug 03, 2013 at 18:21 UTC 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! |
👤 torr0101 🕗 Aug 03, 2013 at 18:24 UTC 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. |
👤 torr0101 🕗 Aug 03, 2013 at 20:56 UTC Actually, testing reveals that the builder does not chop off Accessor prefixes: public class FooTest
} Also, the builder's visibility appears to be package level. Is this required? |
👤 middelkoop 🕗 Aug 05, 2013 at 12:23 UTC 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. |
👤 mofleury 🕗 Aug 28, 2013 at 06:51 UTC 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? |
👤 reinierz 🕗 Aug 28, 2013 at 08:06 UTC 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. |
👤 reinierz 🕗 Aug 28, 2013 at 08:09 UTC 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:
Whereas the useful meaning is:
I'd have to be called 'skipMethod' or some such. |
👤 mofleury 🕗 Aug 28, 2013 at 14:25 UTC 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...
Anyway, I know now that I can quit hoping to have safe builders in mainstream lombok, thank you for this answer. |
👤 david.joaquim 🕗 Sep 06, 2013 at 15:09 UTC 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. 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. 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'". |
👤 Maaartinus 🕗 Sep 07, 2013 at 23:49 UTC 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.
@ Builder(required=true) @ AllArgsConstructor X wrong(int x) { The syntax could be like follows:
I'm not advocating this, just pointing to the possibilities. |
End of migration |
Migrated from Google Code (issue 16)
The text was updated successfully, but these errors were encountered: