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

Ensure DSL for Exacts #13

Merged
merged 4 commits into from
May 16, 2023
Merged

Ensure DSL for Exacts #13

merged 4 commits into from
May 16, 2023

Conversation

ustitc
Copy link
Collaborator

@ustitc ustitc commented May 11, 2023

  • Added ensure dsl for Exact
  • Fixed the directory naming (files were in directory arrow.exact by my mistake)

Decided to use ensure instead of refine, I think it will be better for users to have consistent naming throughout all Arrow modules. ensure accepts other Exact so now it must be easier to combine them.

The only problem is that I am not quite familiar with knit and probably have made something wrong, @nomisRev please fix me in that case 🙏

Feel free to change this PR!

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.

Excellent work @ustits! 👏 Can't wait to use it in my projects

I support the name change to ensure to keep it consistent with Arrow.

@ILIYANGERMANOV ILIYANGERMANOV mentioned this pull request May 14, 2023
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.

This looks super awesome @ustits 🙌 Great work, I only have two small nits, so I am going to approve. We can update or change them later if we want to ☺️

Feel free to merge 🥳

PS: Not much to know about Knit, it seems you did great otherwise the build would've failed. It just makes sure that the code examples we write are valid. Then you run ./gradlew knit to update the tests or it fails on CI saying the knit is outdated.

override fun from(value: A): Either<E, R> = either { construct(ExactScope(value, this)) }
}

public class ExactScope<A, E : Any>(public val raw: A, private val raise: Raise<E>) : Raise<E> by raise {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public class ExactScope<A, E : Any>(public val raw: A, private val raise: Raise<E>) : Raise<E> by raise {
public class ExactScope<A, E : Any>(public val raw: A, raise: Raise<E>) : Raise<E> by raise {

Don't think this needs to be captured in case of delegation, right? 🤔

Comment on lines 11 to 13
object : Exact<A, R> {
override fun from(value: A): Either<ExactError, R> = either { construct(ExactScope(value, this)) }
}
Copy link
Member

Choose a reason for hiding this comment

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

Exact can be a fun interface that I look at it like this. It will be more performant, and look slightly better here.

Suggested change
object : Exact<A, R> {
override fun from(value: A): Either<ExactError, R> = either { construct(ExactScope(value, this)) }
}
Exact { value -> either { construct(ExactScope(value, this)) } }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right, also will change ExactEither

@ustitc ustitc merged commit 957cf86 into main May 16, 2023
4 checks passed
@nomisRev nomisRev deleted the ensure_for_exacts branch May 16, 2023 06:28
@nomisRev
Copy link
Member

Thank you @ustits 🙌

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.

None yet

3 participants