Return to Answer

I disagree with your assertion that it is a red flag. I also disagree with your representation of the counterpoint, that there is One Right Way of representing a Builder pattern.

To me there's one question: Is the Builder a necessary part of the type's API? Here, is the PersonBuilder a necessary part of the Person API?

It is entirely reasonable that a validity-checked, immutable, and/or tightly-encapsulated Person class will necessarily be created through a Builder it provides (regardless of whether that builder is nested or adjacent). In doing so, the Person can keep all of its fields private or package-private and final, and also leave the class open for modification if it is a work in progress. (It may end up closed for modification and open for extension, but that's not important right now, and is especially controversial for immutable data objects.)

If it is the case that a Person is necessarily created through a supplied PersonBuilder as part of a single package-level API design, then a circular coupling is just fine and a red flag isn't warranted here. Notably, you stated it as an incontrovertible fact and not an API design opinion or choice, so that may have contributed to a bad response. I wouldn't have downvoted you, but I don't blame someone else for doing so, and I'll leave the rest of the discussion of "what warrants a downvote" to the Code Review help center and meta. Downvotes happen; it's no "slap".

Of course, if you want to leave a lot of ways open to build a Person object, then the PersonBuilder could become a consumer or utility of the Person class, on which the Person should not directly depend. This choice seems more flexible—who wouldn't want more object creation options?—but in order to hold to guarantees (like immutability or serializability) suddenly the Person has to expose a different kind of public creation technique, such as a very long constructor. If the Builder is meant to represent or replace a constructor with a lot of optional fields or a constructor open to modification, then the class's long constructor may be an implementation detail better left hidden. (Also bear in mind that an official and necessary Builder doesn't preclude you from writing your own construction utility, just that your construction utility may consume the Builder as an API as in my original question above.)

p.s.: I note that the code review sample you listed has an immutable Person, but the counterexample from Wikipedia lists a mutable Car with getters and setters. It may be easier to see the necessary machinery omitted from the Car if you keep the language and invariants consistent between them.

I disagree with your assertion that it is a red flag. I also disagree with your representation of the counterpoint, that there is One Right Way of representing a Builder pattern.

To me there's one question: Is the Builder a necessary part of the type's API? Here, is the PersonBuilder a necessary part of the Person API?

It is entirely reasonable that a validity-checked, immutable, and/or tightly-encapsulated Person class will necessarily be created through a Builder it provides (regardless of whether that builder is nested or adjacent). In doing so, the Person can keep all of its fields private or package-private and final, and also leave the class open for modification if it is a work in progress. (It may end up closed for modification and open for extension, but that's not important right now, and is especially controversial for immutable data objects.)

If it is the case that a Person is necessarily created through a supplied PersonBuilder as part of a single package-level API design, then a circular coupling is just fine and a red flag isn't warranted here. Notably, you stated it as an incontrovertible fact and not an API design opinion or choice, so that may have contributed to a bad response. I wouldn't have downvoted you, but I don't blame someone else for doing so, and I'll leave the rest of the discussion of "what warrants a downvote" to the Code Review help center and meta. Downvotes happen; it's no "slap".

Of course, if you want to leave a lot of ways open to build a Person object, then the PersonBuilder could become a consumer or utility of the Person class, on which the Person should not directly depend. This choice seems more flexible—who wouldn't want more object creation options?—but in order to hold to guarantees (like immutability or serializability) suddenly the Person has to expose a different kind of public creation technique, such as a very long constructor. If the Builder is meant to represent or replace a constructor with a lot of optional fields or a constructor open to modification, then the class's long constructor may be an implementation detail better left hidden. (Also bear in mind that an official and necessary Builder doesn't preclude you from writing your own construction utility, just that your construction utility may consume the Builder as an API as in my original question above.)

p.s.: I note that the code review sample you listed has an immutable Person, but the counterexample from Wikipedia lists a mutable Car with getters and setters. It may be easier to see the necessary machinery omitted from the Car if you keep the language and invariants consistent between them.

I disagree with your assertion that it is a red flag. I also disagree with your representation of the counterpoint, that there is One Right Way of representing a Builder pattern.

To me there's one question: Is the Builder a necessary part of the type's API? Here, is the PersonBuilder a necessary part of the Person API?

It is entirely reasonable that a validity-checked, immutable, and/or tightly-encapsulated Person class will necessarily be created through a Builder it provides (regardless of whether that builder is nested or adjacent). In doing so, the Person can keep all of its fields private or package-private and final, and also leave the class open for modification if it is a work in progress. (It may end up closed for modification and open for extension, but that's not important right now, and is especially controversial for immutable data objects.)

If it is the case that a Person is necessarily created through a supplied PersonBuilder as part of a single package-level API design, then a circular coupling is just fine and a red flag isn't warranted here. Notably, you stated it as an incontrovertible fact and not an API design opinion or choice, so that may have contributed to a bad response. I wouldn't have downvoted you, but I don't blame someone else for doing so, and I'll leave the rest of the discussion of "what warrants a downvote" to the Code Review help center and meta. Downvotes happen; it's no "slap".

Of course, if you want to leave a lot of ways open to build a Person object, then the PersonBuilder could become a consumer or utility of the Person class, on which the Person should not directly depend. This choice seems more flexible—who wouldn't want more object creation options?—but in order to hold to guarantees (like immutability or serializability) suddenly the Person has to expose a different kind of public creation technique, such as a very long constructor. If the Builder is meant to represent or replace a constructor with a lot of optional fields or a constructor open to modification, then the class's long constructor may be an implementation detail better left hidden. (Also bear in mind that an official and necessary Builder doesn't preclude you from writing your own construction utility, just that your construction utility may consume the Builder as an API as in my original question above.)

p.s.: I note that the code review sample you listed has an immutable Person, but the counterexample from Wikipedia lists a mutable Car with getters and setters. It may be easier to see the necessary machinery omitted from the Car if you keep the language and invariants consistent between them.

I disagree with your assertion that it is a red flag. I also disagree with your representation of the counterpoint, that there is One Right Way of representing a Builder pattern.

To me there's one question: Is the Builder a necessary part of the type's API? Here, is the PersonBuilder a necessary part of the Person API?

It is entirely reasonable that a validity-checked, immutable, and/or tightly-encapsulated Person class will necessarily be created through a Builder it provides (regardless of whether that builder is nested or adjacent). In doing so, the Person can keep all of its fields private or package-private and final, and also leave the class open for modification if it is a work in progress. (It may end up closed for modification and open for extension, but that's not important right now, and is especially controversial for immutable data objects.)

If it is the case that a Person is necessarily created through a supplied PersonBuilder as part of a single package-level API design, then a circular coupling is just fine and a red flag isn't warranted here. Notably, you stated it as an incontrovertible fact and not an API design opinion or choice, so that may have contributed to a bad response. I wouldn't have downvoted you, but I don't blame someone else for doing so, and I'll leave the rest of the discussion of "what warrants a downvote" to the Code Review help center and meta. Downvotes happen; it's no "slap".

Of course, if you want to leave a lot of ways open to build a Person object, then the PersonBuilder could become a consumer or utility of the Person class, on which the Person should not directly depend. This choice seems more flexible—who wouldn't want more object creation options?—but in order to hold to guarantees (like immutability or serializability) suddenly the Person has to expose a different kind of public creation technique, such as a very long constructor. If the Builder is meant to represent or replace a constructor with a lot of optional fields or a constructor open to modification, then the class's long constructor may be an implementation detail better left hidden. (Also bear in mind that an official and necessary Builder doesn't preclude you from writing your own construction utility, just that your construction utility may consume the Builder as an API as in my original question above.)

p.s.: I note that the code review sample you listed has an immutable Person, but the counterexample from Wikipedia lists a mutable Car with getters and setters. It may be easier to see the necessary machinery omitted from the Car if you keep the language and invariants consistent between them.

I disagree with your assertion that it is a red flag. I also disagree with your representation of the counterpoint, that there is One Right Way of representing a Builder pattern.

To me there's one question: Is the Builder a necessary part of the type's API? Here, is the PersonBuilder a necessary part of the Person API?

It is entirely reasonable that a validity-checked, immutable, and/or tightly-encapsulated Person class will necessarily be created through a Builder it provides (regardless of whether that builder is nested or adjacent). In doing so, the Person can keep all of its fields private or package-private and final, and also leave the class open for modification if it is a work in progress. (It may end up closed for modification and open for extension, but that's not important right now, and is especially controversial for immutable data objects.)

If it is the case that a Person is necessarily created through a supplied PersonBuilder as part of a single package-level API design, then a circular coupling is just fine and a red flag isn't warranted here. Notably, you stated it as an incontrovertible fact and not an API design opinion or choice, so that may have contributed to a bad response. I wouldn't have downvoted you, but I don't blame someone else for doing so, and I'll leave the rest of the discussion of "what warrants a downvote" to the Code Review help center and meta. Downvotes happen; it's no "slap".

Of course, if you want to leave a lot of ways open to build a Person object, then the PersonBuilder becomes a consumer or utility of the Person class, upon which the Person should not directly depend. This choice seems more flexible—who wouldn't want more object creation options?—but in order to hold to guarantees (like immutability or serializability) suddenly the Person has to expose a different kind of public creation technique, such as a very long constructor. If the Builder is meant to represent a constructor with a lot of optional fields or a constructor open to modification, this long constructor may be an implementation detail better left hidden.

(p.s.: I note that the code review sample you listed has an immutable Person, but the counterexample from Wikipedia lists a mutable Car with getters and setters. It may be easier to see the necessary machinery omitted from the Car if you keep the language and invariants consistent between them.)

I disagree with your assertion that it is a red flag. I also disagree with your representation of the counterpoint, that there is One Right Way of representing a Builder pattern.

To me there's one question: Is the Builder a necessary part of the type's API? Here, is the PersonBuilder a necessary part of the Person API?

It is entirely reasonable that a validity-checked, immutable, and/or tightly-encapsulated Person class will necessarily be created through a Builder it provides (regardless of whether that builder is nested or adjacent). In doing so, the Person can keep all of its fields private or package-private and final, and also leave the class open for modification if it is a work in progress. (It may end up closed for modification and open for extension, but that's not important right now, and is especially controversial for immutable data objects.)

If it is the case that a Person is necessarily created through a supplied PersonBuilder as part of a single package-level API design, then a circular coupling is just fine and a red flag isn't warranted here. Notably, you stated it as an incontrovertible fact and not an API design opinion or choice, so that may have contributed to a bad response. I wouldn't have downvoted you, but I don't blame someone else for doing so, and I'll leave the rest of the discussion of "what warrants a downvote" to the Code Review help center and meta. Downvotes happen; it's no "slap".

Of course, if you want to leave a lot of ways open to build a Person object, then the PersonBuilder could become a consumer or utility of the Person class, on which the Person should not directly depend. This choice seems more flexible—who wouldn't want more object creation options?—but in order to hold to guarantees (like immutability or serializability) suddenly the Person has to expose a different kind of public creation technique, such as a very long constructor. If the Builder is meant to represent or replace a constructor with a lot of optional fields or a constructor open to modification, then the class's long constructor may be an implementation detail better left hidden. (Also bear in mind that an official and necessary Builder doesn't preclude you from writing your own construction utility, just that your construction utility may consume the Builder as an API as in my original question above.)

p.s.: I note that the code review sample you listed has an immutable Person, but the counterexample from Wikipedia lists a mutable Car with getters and setters. It may be easier to see the necessary machinery omitted from the Car if you keep the language and invariants consistent between them.