inferred / FreeBuilder

Automatic generation of the Builder pattern for Java
Apache License 2.0
832 stars 101 forks source link

Alternative name for build method #432

Closed neilswingler closed 3 years ago

neilswingler commented 4 years ago

It could be useful, especially when migrating legacy hand coded builders to Freebuilder, to be able to have the build() method have a different name.

e.g. by declaring the alternative name in the Builder

public Foo buildFoo() {
  return super.buildFoo();
}
alicederyn commented 4 years ago

Is it necessary to not have the build method present? Otherwise you could just declare the legacy method in addition:

public Foo buildFoo() {
  return build();
}
neilswingler commented 4 years ago

In my use case I want to overload the actual build() method to return a different type so I require the generated builder to not declare a build() method at all.

I have a mutable data object with getters/setters and a hand coded builder that I hope will eventually be migrated to an immutable Freebuilder implementation but as a stepping stone I want to at least be able to use FreeBuilder for the builder part.

alicederyn commented 4 years ago

Can you post a simplified example of what that API would look like?

There are two methods on builders with the same signature (no parameters, returns a Foo), build and buildPartial, and while I can't think of any others users might want to add, it would be API-breaking to stop emitting build if one were added — but depending on the exact pattern you're making, it may still be possible to reliably detect it without breaking other use cases.

neilswingler commented 4 years ago

Could this work? It would not be a breaking change because currently, overloading the build() method return type leads to a compiler error.

@Freebuilder
interface Foo {
    class Builder extends Foo_Builder {
        public Bar build() { ... }  // incompatible return type => don't emit a build()
        public Foo buildFoo() { // compatible return type, emit buildFoo instead
            return super.buildFoo();
        |
    }
}
alicederyn commented 4 years ago

I would prefer to find a non-obscured method name matching a specific pattern for selecting the build method — perhaps this:

1) If the build() method is protected or package-protected on the subclass, follow suit 2) If the build() method is private or signature-incompatible, instead generate a package-protected _buildImpl() method for the subclass to call 3) Optional for first implementation If that method is already present and incompatible, continue trying with _buildImpl2() etc. until a suitable candidate is found

This then cleanly extends to other methods in future, e.g. buildPartial() becomes _buildPartialImpl()

That would make your code look like:

@FreeBuilder
interface Foo {
    class Builder extends Foo_Builder {
        public Bar build() { ... }  // incompatible return type => FreeBuilder emits _buildImpl instead
        public Foo buildFoo() {
            return super._buildImpl();
        }
    }
}

I would be happy to accept a PR implementing this feature. If you are interested in doing this, let me know and I'll point you to the bits of the code you'd need to fit it into.

neilswingler commented 4 years ago

Thanks for giving it the consideration Alice. I'm off on holiday now but hopefully might have some time in September to try to make a pr.

On Fri, 31 Jul 2020, 11:28 Alice, notifications@github.com wrote:

I would prefer to find a non-obscured method name matching a specific pattern for selecting the build method — perhaps this:

  1. If the build() method is protected or package-protected on the subclass, follow suit
  2. If the build() method is private or signature-incompatible, instead generate a package-protected _buildImpl() method for the subclass to call
  3. Optional for first implementation If that method is already present and incompatible, continue trying with _buildImpl2() etc. until a suitable candidate is found

This then cleanly extends to other methods in future, e.g. buildPartial() becomes _buildPartialImpl()

That would make your code look like:

@FreeBuilder interface Foo {

class Builder extends Foo_Builder {

    public Bar build() { ... }  // incompatible return type => FreeBuilder emits _buildImpl instead

    public Foo buildFoo() {

        return super._buildImpl();

    }

}

}

I would be happy to accept a PR implementing this feature. If you are interested in doing this, let me know and I'll point you to the bits of the code you'd need to fit it into.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/inferred/FreeBuilder/issues/432#issuecomment-667028507, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMYAHZQQBGYMJFZI7BJXB73R6KFDNANCNFSM4PKLQMEA .

alicederyn commented 4 years ago

Gmail doesn't notify me of forum messages, so if you do pick it up, feel free reach out to me directly by email for a quicker response. Meanwhile, enjoy your holiday!

alicederyn commented 3 years ago

Hopefully FreeBuilder v2.7.0 will solve this issue for you. Please do raise another issue if it does not!

neilswingler commented 3 years ago

Thanks a lot. I tried it out already.