projectlombok / lombok

Very spicy additions to the Java programming language.
https://projectlombok.org/
Other
12.78k stars 2.37k forks source link

[FEATURE] provide @Builder.Exclude #2307

Closed nimo23 closed 4 years ago

nimo23 commented 4 years ago

Describe the feature We can use the @Builder on class level. However, with this, we cannot omit some fields from the builder. Please provide the field annotation @Builder.Exclude to exclude fields from the builder pattern.

@Builder
public class User {

    // should not be included in the builder pattern
    @Builder.Exclude
    private int id;

    private String name;

}
rspilker commented 4 years ago

I do miss in what common scenario you would want to exclude fields from the builder.

The way to solve this is by putting @Builder on a constructor instead of a type.

Basically, you can put @Builder on a method, and we will generate a Builder for all parameters, where the build() invokes the method. In this sense, the constructor is also a method.

For convenience, we allow @Builder on a type. Then we generate an @AllArgsConstructor, and act as if that was annotated with @Builder.

nimo23 commented 4 years ago

@rspilker

I do miss in what common scenario you would want to exclude fields from the builder.

Look at this class:

@Builder
@NoArgsConstructor
@AllArgsConstructor
public class User {

// many properties
...

@Builder.Default
private Task task = new Task();

@Builder.Exclude // does not exist
private Type type;

}

For convenience, we allow @Builder on a type. Then we generate an @AllArgsConstructor, and act as if that was annotated with @Builder.

Note that @AllArgsConstructor is only created if no @NoArgsConstructor is on the class. See https://github.com/rzwitserloot/lombok/issues/2305.

@Builder on a constructor or on method

yes, but with a class having many properties we have to maintain a large constructor or large parameter list.

nimo23 commented 4 years ago

@rspilker look at https://stackoverflow.com/questions/30717640/how-to-exclude-property-from-lombok-builder then you understand the use cases why having a @Builder.Exclude makes sense.

Actually, If I have many properties within a class and only want to exclude one property from the builder then I am forced to put a large constructor or static method with @Builder annoation instead of having the @Builder on class level. With @Builder.Exclude I do not need to add a constructor or static method manually.

@rspilker So please reopen this issue and make @Builder.Exclude available. Would be great. This annotation can be very convenient.

eliast101 commented 2 years ago

This issue needs to be reopened.

rzwitserloot commented 2 years ago

@eliast101 No.

Write a constructor that omits the field you want to exclude, and stick @Builder on that.

Please note that we are almost impervious to peer pressure and will disregard popularity contests as a consequence. If you'd like us to make a different decision, you're going to have to come up with an actual argument.

Kyle-Holub commented 2 years ago

Lombok exists to provide convenience. Yeah, we can slap @Builder on a constructor with 20 arguments, but what we are saying is we want to avoid doing precisely that - rather it would be easier if we could simply exclude with an annotation. The argument being made here is Lombok would better support its objective of convenience if it had this feature.

PSA for everyone else looking for solutions - I've encountered use cases where I don't want certain fields exposed with @SuperBuilder. I've had the best luck stepping into the generated Lombok class, copy the builder method I do not want public, and paste it as private in the source code. Less code than a giant constructor.

dkelly103 commented 2 years ago

I would have a preference for @Builder.Exclude too, but here's an alternative to applying @Builder to large constructors:

@Builder
public class DataModel {

    private String fieldOne;
    private String fieldTwo;

    public static class DataModelBuilder(){

        private DataModelBuilder fieldTwo(String fieldTwo){
            return this;
        }

    }
}

It essentially overrides the fieldTwo builder function to make it private and unusable

CrypticCabub commented 2 years ago

Ability to generate a 'mostArgsConstructor' and put a builder on that on the fly would be very very useful. Primary use-case would be on JPA entities with an id attribute. The id is typically maintained by the underlying DataSource implementation and should never be set explicitly as a result.

While creating a manual constructor that omits the id but includes everything else does work, it defeats much of the benefit of using lombok on what is essentially a data class that includes an id for database access purposes

Rather than a @Builder.exclude (which doesn't make sense with an @allArgsConstructor) annotation, would there be a good argument for @Lombok.exclude instead which will exclude the field from both the @allArgsConstructor and @Builder annotations on the class level?

vkazar commented 1 year ago

I would have a preference for @Builder.Exclude too, but here's an alternative to applying @Builder to large constructors:

@Builder
public class DataModel {

    private String fieldOne;
    private String fieldTwo;

    public static class DataModelBuilder(){

        private DataModelBuilder fieldTwo(String fieldTwo){
            return this;
        }

    }
}

It essentially overrides the fieldTwo builder function to make it private and unusable

I have 13 fields. Some of them are for initiation, some of them are technical. @Builder.Exclude is needed

remal commented 1 year ago

@rzwitserloot please consider reopening the issue.

My case: a stateful component that has immutable configuration and mutable fields for the business logic. These mutable fields should not be exposed to constructor/builder. However, the number of "config" fields is relatively big (11 right now), so writing a constructor is not a convenient solution.

class Component {

    private final int config1;
    private final String config2;
    // ...

    private int mutable1;

    public void doLogic() {
        if (mutable1++ >= config1) { // <- EXAMPLE
            // do something
        }
    }

}
JakeHenry commented 1 year ago

This feature request needs to be re-opened. The arguments above are more than sufficient. Not all fields of a class implementing the builder pattern are required to be exposed. Forcing people to implement a massive constructor with 1-N arguments is error prone, makes code maintenance more difficult defeating the purpose of using lombok, and will likely violate a lot of Linter and checkstyle rules for developers.

tiparega commented 1 year ago

So @rspilker, you're basically saying that is better to write this:

@Getter
public class Numbers {
    private int one;
    private int two;
    private int three;
    private int four;
    private int five;
    private int six;
    private int seven;
    private int eight;
    private int nine;
    private int ten;
    private int eleven;
    private int twelve;
    private int calculatedOne;

    @Builder
    public Numbers(int one, int two, int three, int four, int five, int six, int seven, int eight, int nine, int ten,
            int eleven, int twelve) {
        super();
        this.one = one;
        this.two = two;
        this.three = three;
        this.four = four;
        this.five = five;
        this.six = six;
        this.seven = seven;
        this.eight = eight;
        this.nine = nine;
        this.ten = ten;
        this.eleven = eleven;
        this.twelve = twelve;
    }
}

Than this:

@Getter
@Builder
public class Numbers {
    private int one;
    private int two;
    private int three;
    private int four;
    private int five;
    private int six;
    private int seven;
    private int eight;
    private int nine;
    private int ten;
    private int eleven;
    private int twelve;

    @Builder.Exclude //Doesn't exist yet
    private int calculatedOne;
}

I don't see how can first option be better than second one. As pointed by other users, it's boilerplate, it's error prone, the constructor will generate an alert on code scanners, etc. Of course we can write a constructor, we can also write getters and setters, but we want to avoid it. That's the reason for using Lombok. Also, Builder.Default can't be used and there is no way to know if it was setted or not (if you want an int fiel default to 3 but intentionally use the builder to set value 0, it's impossible to distinguish from not set).

@rzwitserloot I think these are valid arguments. what are yours?

eric-creekside commented 11 months ago

I find it interesting that the Lombok developers insisted on arguments and use cases to justify the feature, and yet when presented with those, have yet to present any argument against it. Having worked in open-source projects for many years myself, I understand the need to avoid "build all the features!" However, to simply dismiss valid feature requests without legitimate discussion of the merits and downsides, isn't being a good open-source citizen, either. There has to be a middle ground and an honest discussion. Perhaps the developers would respond to a PR to implement this?

CrypticCabub commented 11 months ago

my preference is still an @Lombok.exclude (excludes from @allArgsConstructor as well -- see my prev comment). To give the benefit of the doubt to the developers, there are 800 open issues right now and this one might have just fallen under the radar. maybe a new ticket referencing this one for extra visibility might help?

Rawi01 commented 11 months ago

@CrypticCabub It was obviously considered but rejected because there was no comon scenario mentioned in the issue. Creating a new issue doesn't increase visibility.

@JakeHenry Instead of assuming negative intentions, complaining about infantile mindsets and adding useless comments that this feature is important you could try to collect more evidence that this annotation is really needed. This should also include an explanation why you need the additional field and why it has to be excluded.

eric-creekside commented 11 months ago

At least 8 people have responded since the issue was opened, with reasoning and justification. Considering the small percentage of users who ever bother to contribute via issues or discussions, that's a pretty significant turnout. The justification has been established, there is no debating that. Now we are just waiting for a development team to respond sincerely. Nobody is going to spend the time creating a PR if the developers are just going to ignore or reject it without due consideration.

JakeHenry commented 11 months ago

@Rawi01 How on Earth do you read the comments above and conclude there is no common scenario mentioned here? Asking why people need certain fields in their business logic is outside the scope of the details you should be asking for.

Rawi01 commented 11 months ago

I never asked for business logic but that might also help you to create a detailed usage example. A simple "I have a class with X fields and one of them should be excluded in my builder" will probably not magically change Reinier's or Roels' mind. But an additional "because it should be impossible to set the ID of my JPA entity" adds useful context and illustrates why this feature might be important to more than a few people.

remal commented 11 months ago

@Rawi01 isn't the example I provided like this?

CrypticCabub commented 11 months ago

the JPA ID example seems like a pretty major one imo. maybe not enough by itself to get an exclusion feature prioritized (I know what it's like having way more FRs than time), but big enough to not close the feature as a won't do?

JakeHenry commented 11 months ago

It's clear that there is already an understanding of why this feature is being requested. I'm not going to participate in the fishing for an unending quantity of examples when the subject matter provided above is more than sufficient. However, I will share this example of a workaround that worked for me that hopefully will help those that don't have time to jump through hoops. I understand that this workaround may not work for some people's use cases.

The basic concept is to create an inner class that holds all fields you don't want exposed to the builder.

One might be tempted to say "well you should just refactor your code base so that you don't need to have the unexposed fields". In some cases you can and in some cases you can't. It's not an across the board assumption you can make.

@Builder(toBuilder = true)
@Getter
@Setter
@ToString
@EqualsAndHashCode
@AllArgsConstructor
public class PublicData
{
    protected int configurationData1;

    protected int configurationData2;

    protected int configurationData3;

    //100 other exposed configuration fields
    //...
    //...

    @Delegate(excludes = BusinessData.class) // No business data delegates will be generated
    @Getter(AccessLevel.NONE) // No one can access business data via getter
    @ToString.Exclude // <-Wow! What a concept!
    @EqualsAndHashCode.Exclude //<-Wow! What a concept!
    // final instance field initialized here prevents setter generation and inclusion in the builder pattern
    private final BusinessData businessData = new BusinessData();

    /**
     * This is private business logic
     */
    public void doBusinessLogic()
    {
        // Business variables and actions are recalculated before doing any logic
        updateDependentBusinessData();

        // Do business logic - the dependent business fields are now updated
        if (businessData.independentBusinessData1)
        {
            System.out.println(businessData.dependentBusinessData1);
        }
    }

    /**
     * This private instance method recalculates the values of any private business logic fields not exposed to the builder pattern that
     * are dependent on the exposed builder fields.
     */
    private void updateDependentBusinessData()
    {
        businessData.dependentBusinessData1 = configurationData1 + configurationData2 + configurationData3;
        System.out.printf("Recalculating dependent business logic fields = %d + %d + %d = %d%n", configurationData1, configurationData2, configurationData3, businessData.dependentBusinessData1);
    }

    /**
     * Internal container class that holds private business data fields that are not to be exposed in any lombok builder constructs
     */
    @ToString
    @EqualsAndHashCode
    static class BusinessData
    {
        private int dependentBusinessData1;

        private boolean independentBusinessData1 = true;
    }
}
dev-tori commented 5 months ago

I sincerely appreciate the hard work and dedication of all the developers who have contributed to this project. It's hard to imagine where this crucial open-source project, widely used around the world, would be without your passionate involvement. However, I am somewhat surprised and disappointed that this issue remains unresolved and closed as of 2024. I wonder if there's any possibility for further discussion or reconsideration on this matter.

alex-kolesnikov commented 5 months ago

I need @Builder.Exclude too :)

javawithrocky commented 4 months ago

Same here; dearly need it. The class has 20+ properties and I want to exclude couple. Plus the comment about peer pressure was totally unnecessary. There are ample examples provided why it's needed.

me-12 commented 3 months ago

I would also like to have such a feature

lmartelli commented 3 months ago

For the record, also needed here.

mr-robert commented 3 months ago

Also could really use this here, specifically for @SuperBuilder to exclude a certain field... unless Lombok could achieve it automatically that would be nice.

For example:

@SuperBuilder
public class Base {
   private final String name;
   private final String value;
}
@SuperBuilder
public class NamedBase extends Base {
   @Builder.Exclude
   private final String name = "hardcoded name";
   private final String additionalData;
}

My base class may serve as a kind of interface, where I need certain values to be provided, but in certain cases/overrides, those values may always be the same and therefore annoying and redundant to repeat. I dont want NamedBase.builder().name(...) to appear, but it does because it can not be excluded easily from SuperBuilder even when adding syntax like private final ... = "hardcoded value" that normally stops a builder from generating it? Perhaps the real solution is that Lombok should detect the hardcoded value in the child class and exclude?

MarekMalevic commented 2 months ago

For my project the @Builder.Exclude is critical so I can use @SuperBuilder. At the moment @SuperBuilder exposes every single field of every class in the hierarchy which makes the final builder useless.

psabharwal123 commented 2 months ago

i need @Builder.Exclude too

H-Lo commented 1 month ago

Now I'm fecked because I cannot use @Builder anymore because I introduced a local var. Previously there was no need for construcor because Spring Boot used @Builder generated code and I was set free from extremely ugly constructor which is bloating my code.

@Service @Builder
public class AdminServiceImpl implements AdminService {

    private final ArticleRepository      articleRepository;
    private final ArticleService         articleService;
    private final CaffeineCacheManager   caffeineCacheManager;
    private final CustomProperties       customProperties;
    private final LanguageRepository     languageRepository;
    private final ManufacturerRepository manufacturerRepository;
    private final PhotoService           photoService;
    private final PriceInfoService       priceInfoService;
    private final PriceUpdateService     priceUpdateService;
    private final ProviderRepository     providerRepository;
    private final UserRepository         userRepository;
    private final PhotoRepository        photoRepository;

    // local var as a flag - EXCLUDE HERE NEEDED
    private boolean updatingPhotos = false;

}
***************************
APPLICATION FAILED TO START
***************************

Description:
Parameter 12 of constructor in com.thevegcat.app.web.admin.service.AdminServiceImpl required a bean of type 'boolean' that could not be found.

Action:
Consider defining a bean of type 'boolean' in your configuration.

This is the constructor that I have to put in my code because there is no EXCLUDE option:

public AdminServiceImpl(
    final ArticleRepository      articleRepository,
    final ArticleService         articleService,
    final CacheManager           cacheManager,
    final CustomProperties       customProperties,
    final LanguageRepository     languageRepository,
    final ManufacturerRepository manufacturerRepository,
    final PhotoService           photoService,
    final PriceInfoService       priceInfoService,
    final PriceUpdateService     priceUpdateService,
    final ProviderRepository     providerRepository,
    final UserRepository         userRepository,
    final PhotoRepository        photoRepository
) {
    this.articleRepository      = articleRepository;
    this.articleService         = articleService;
    this.caffeineCacheManager   = (CaffeineCacheManager) cacheManager;
    this.customProperties       = customProperties;
    this.languageRepository     = languageRepository;
    this.manufacturerRepository = manufacturerRepository;
    this.photoService           = photoService;
    this.priceInfoService       = priceInfoService;
    this.priceUpdateService     = priceUpdateService;
    this.providerRepository     = providerRepository;
    this.userRepository         = userRepository;
    this.photoRepository        = photoRepository;
}

Pretty sad, isn't it?

CrypticCabub commented 1 month ago

this is more justification for a feature than I have ever seen on any other project I've worked on. Is there a reason this is being ignored?

a1shadows commented 1 month ago

To add to the general consensus, I also need this for a project I'm working on.

Ordiel commented 3 weeks ago

I add myself to those needing such annotation; in my case I also need it for JPA yet in this case is to prevent setting 1:N relations, in JPA it is possible to have references to other tables which don't necessarily match to a field itself, those are lazy initialized and depend on the relations found on the DB, it should not be allowed by developers to neither define nor modify such relations, for objects retrieved from the DB it is as simple as @Setter(access=PRIVATE) yet when creating new instances I am still able to add those through the builder...

I could elaborate further on it yet I am not sure how much time do I want to spend on that if its just gonna be dismissed

I don't think it could be that complex to exclude an attribute from the builder with the understatement that such will be set to either null or the default value for primitive attributes

a1shadows commented 1 week ago

@rspilker @rzwitserloot I fully agree that the final decision regarding the direction of the project lies solely on the maintainers and greatly respect the effort that goes into maintaining such high standards.

Since you guys are open to arguments as to why this feature may be needed, I thought to summarize everything up to this point:

  1. The feature was requested with the need being convenience since needing to exclude a field from a class builder is a very common pattern. Not all fields in a class need be available to the user to tinker.
  2. You guys pointed out that internally the builder runs on a generated all args constructor when annotated on a type and that as such any method(such as one that excludes the fields that need exclusion) can be annotated with @Builder so in this way the library definitely provides an option to exclude fields.
  3. The general counter to this on this thread is that most people use lombok to avoid boilerplate code for common access/creation patterns and creating a constructor/method with possibly a large number of fields just to exclude a few and then to annotate it with builder is not as convenient as could be. JPA was also mentioned as one of the use-cases that could really benefit from excluding fields.

I haven't seen a counter to the third point up to this point. What I can think of (possibly incorrectly) is that this change breaks the "pattern" of having Builder mimic/use a method underneath without introducing side effects to it. Having a @Builder.Exclude would definitely introduce side effects and increase the overall complexity of the feature and lib.

However, I also see how useful such a feature can be long term if implemented correctly. Even if it does not end up getting implemented, I think we can gain from at least discussing the possible ways in which such a feature could be implemented without hurting the integrity of the project.

Would greatly appreciate your thoughts on this!