nastra edited a comment on pull request #3580: URL: https://github.com/apache/iceberg/pull/3580#issuecomment-975217120
> I don't think that this is a good alternative to the other builder. The things we want to fix with the other builder are to make the builder smarter. It doesn't really help just to have a builder, we want to put more logic in there. @rdblue could you be more specific in what you'd like to see in such a Builder? Then we could check and see whether Immutable builders would support that. The Immutables lib provides more than just generated builders, but if the generated builders don't do what we want them to do, then there are also alternative ways of dealing with this (constructing objects via constructors and not builders, ...). Looking at https://github.com/apache/iceberg/pull/2957/files#diff-c55bb00afe1e6529dc13f2421f18bbb557de1a8111573400d4d3d22df573a9b9R28 I see a straight-forward builder implementation that just sets things. The downside I see with this approach is that you also have to write tests for that builder, since you'd like to make sure that all your fields are properly set and such. I've seen examples in the past where hand-made Builders didn't have unit tests and had bugs in them, so to me it seems it's better to have builders be generated. I also understand that changing something like `TableMetadata` to using Immutables is kind of a critical place with a certain level of uncertainty. I'm perfectly fine with starting at other places in the code if you think the Immutables lib is something that's useful. My personal opinion on using Immutables is that it forces you to adhere to good coding practices, since you're working with Immutable data structures, which brings its own (positive) implications. Additionally, it saves you from writing manual and repetitive code (builders, serializers/deserializers, ...). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
