Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #86, #87
An unsafe version of the API is exposed by using
XBuilderImplin case someone needs it. Technically, the builder stylewithXmethods can be made safe as well, but I feel they're unlikely to be used in Kotlin, so I've omitted them.AssignOncesafety is there to catch simple mistakes, but it can be circumvented incredibly easily with something as simple asif (true) foo = x; foo = x. It also currently doesn't do anything for nested dsls (e.g.PersonBuilder.address), but maybe it could in the future using error deprecation or an opt-in annotation.Another point of contention is that types are made nullable no matter what. That's annoying because someone might assign a field, and immediately try to access it, only to have it be nullable. This could be improved in the future, but it'd have to change the logic in
BuilderImplclasses significantly, and it'd mean that the unsafe version wouldn't have the same semantics as AutoDsl builders do right now (e.g. accessing a required parameter wouldn't give younullif it isn't filled yet).The approach in this PR is incredibly nice and easy to consume in Kotlin (it's my personal favourite). There is an alternative approach that uses type parameters. It would be usable from Java safely (which the current one isn't, with Java consumers having to fall back to the unsafe API), while being slightly more annoying in Kotlin (due to all the type parameters).