... this has actually been discussed already a while ago (1-2 years)... we should be able to find that list of rules related to JPA (the builder is actually not the main concern here).
Concerning validation: validation should happen at the DTO level... and not everywhere like we do currently... that's why frameworks like jakarta.validation exist... if you have to enforce certain values (e. g. non-null) on JPA then why not define these constraints in the database schema (which we actually do)... so, I think this whole argument in favor of static functions to have some sort of validation falls flat here. These static functions are in fact yet another way to mimic constructors and are at the least redundant (if you really want to do it that way then why not just use constructors in the first place)... personally I find them horrible... everyone uses his/her own naming scheme, trying to shorten the input parameters by passing all these wrapper classes is just another way of tying unrelated concepts together (REST/JSON with the persistence layer); I really see no benefit at all in this... and on top of it all: you move the assignment of values yet again a level down in the hierarchy... wouldn't it be nice to just see immediately which values are assigned (and I think the builder pattern is just great for that). I am not sure what point you are trying to make with "limiting the builder’s access to private or package-private" is this really a problem or are we trying to search for one? Let's keep this simple and not overcomplicate things... On Thu, Jul 25, 2024 at 7:21 PM Zeyad Nasef <zeyad.nasef....@gmail.com> wrote: > Hi everyone, > > I hope this message finds you well. > > I would like to discuss the use of the @Builder annotation for entity > classes in Fineract. In a recent pull request #3984 > <https://github.com/apache/fineract/pull/3984/files>, I introduced the > @Builder pattern for entity creation. However, upon further discussion with > @Adam > Saghy <adamsa...@apache.org>, some concerns have emerged that warrant > community feedback: > > *Concerns with Using @Builder for Entities:* > > - Incompatibility with The ORM (interfere with lazy loading, entity > management ...etc.) > - No validation or transformation enforcement when instantiating the > entity. > - Builder pattern might be confusing and let others to set or override > things that should not be. > > *Some Proposed Solutions:* > > - Adding `@NoArgsConstructor` & `@AllArgsConstructor` for resolving > the JPA issues. > - Implementing methods like fullEntryFrom() > > <https://github.com/apache/fineract/blob/develop/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandSource.java#L136> > to encapsulate the creation logic, thereby enforcing validation and > transformations while utilizing the builder pattern to avoid large > constructors with many parameters. > - Limiting the builder’s access to private or package-private scope > and using @Builder.Default to provide specific methods for controlled > instantiation. > > > Your input on this matter would be greatly appreciated. > > Best regards, > Zeyad Nasef >
