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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DISCUSSION] Exact Core 0.0.1 #8
[DISCUSSION] Exact Core 0.0.1 #8
Changes from all commits
f9ad9b9
5690e17
8acee25
7544e44
6e84b5f
65b5c89
50aa234
9cf25cc
e5211f5
c78e69d
8c620fd
0334c72
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit controversial. What do you think?
My opinion is that there are many cases where you're sure that something matches the constraints and going through the hassle of dealing with an
Either
is unnecessary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally think
orThrow
is concise enough it's worth the readability. Kotlin favours explicitness in a lot of areas, and this seems a place that is a good trade-off for it as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's true @nomisRev . IMO, it just becomes too verbose, distracting and not right making the possibility of throwing an exception for cases where that's obviously impossible like:
Personal opinion, the above is too much noise and I prefer the below:
Note these two are artificial examples but in practice we have many places where we're certain that a constraint won't be violated, in my personal projects I reserve the
operator fun invoke(value: A): R = fromOrThrow(value)
for such cases.When using for example
PositiveInt(x + y)
instead ofPositiveInt.fromOrThrow(x + y)
we show our readers that we're certain that in the first casex + y
will always be positive but in the 2nd we direct their attention to knowing that it's not always the case.I suggest leaving it for discussion and re-visiting before releasing the library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make an issue to make a small detekt integration module for this?
This file was deleted.
This file was deleted.