Skip to content
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

Add spec method to Exact #20

Merged
merged 6 commits into from
May 30, 2023
Merged

Add spec method to Exact #20

merged 6 commits into from
May 30, 2023

Conversation

ustitc
Copy link
Collaborator

@ustitc ustitc commented May 28, 2023

  • Changed Exact to abstract class instead of interface Add spec function in ExactEither with context of Raise so that clients can both use implementation and implementation by delegation
    • I suggest that developers won't need to implement/extend their companion objects in most scenarios, so that restriction won't be a big problem
    • Extending from abstract class Implementing feels more "natural" than delegating using by and I think it will be more convenient for clients. But let's keep delegation is a viable alternative
  • Removed ExactScope in favour of extending Raise
    • I suggest that it will be better to limit amount of new abstractions and reuse existing one as much as possible
    • Also removed ExactDsl, I will agree with @CLOVIS-AI in Replace ExactDsl by RaiseDsl #15, adding a separate dsl marker for methods with same semantics will look confusing for clients of library

@CLOVIS-AI
Copy link

Changed Exact to abstract class instead of interface

I don't really see the benefit for this. It makes the DSL more verbose, and adds limitations (e.g. the companion object cannot extend any other class) for no reason I see.

Personally, I'd prefer keeping it as it is now, or letting the user decide (e.g. have a Raise interface and an AbstractRaise abstract class).

Copy link
Collaborator

@ILIYANGERMANOV ILIYANGERMANOV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! Thank you @ustits 👍
@CLOVIS-AI I thought the same at first but the benefit is that your IDE will generate the majority of the code and also it's more obvious than companion object by ....

@ustitc
Copy link
Collaborator Author

ustitc commented May 28, 2023

@CLOVIS-AI @ILIYANGERMANOV Actually I figured out how we can keep both approaches. I turned Exact back to interface and since it is a functional interface we can instantiate it like this:

@JvmInline
value class NotBlankString private constructor(val value: String) {
   companion object : Exact<String, NotBlankString> by Exact({
     ensure(it.isNotBlank()) { ExactError("Cannot be blank.") }
     NotBlankString(it)
   })
}

@nomisRev I will need help, I added one more example to Exact but the last one is not showing in documentation in IDE, can't understand why

image

@ustitc ustitc changed the title Change Exact to abstract class Add spec method to Exact May 28, 2023
@CLOVIS-AI
Copy link

I turned Exact back to interface and since it is a functional interface

Oh, that's great. This way, we can explain to users how it works using the regular interface implementation using override, and later in the documentation introduce them to the by sugar.

Copy link
Member

@nomisRev nomisRev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nomisRev I will need help, I added one more example to Exact but the last one is not showing in documentation in IDE, can't understand why

@ustits I think this has been resolved in 0348de6?

Amazing work all! I cannot believe how incredible this DSL looks, and the underlying code is really simple 🙌

Thanks all for collaborating so amazingly 😍

src/commonMain/kotlin/arrow/exact/ExactDsl.kt Outdated Show resolved Hide resolved
ustitc and others added 2 commits May 30, 2023 20:59
Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com>
@ustitc
Copy link
Collaborator Author

ustitc commented May 30, 2023

@nomisRev thanks for the feedback! Actually the problem was in using @JvmInline annotation inside <!--- INCLUDE --> directive, fixed it

@ustitc ustitc merged commit 07ec53d into main May 30, 2023
@ustitc ustitc deleted the exact_as_abstract_class branch May 30, 2023 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants