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 instance lookup for Foldable and Traverse methods #129

Merged
merged 4 commits into from
Jul 17, 2017

Conversation

pakoito
Copy link
Member

@pakoito pakoito commented Jul 17, 2017

This PR converts some methods that require typeclass instances into inline extension functions that allow for implicit lookup using our helpers.

@codecov-io
Copy link

codecov-io commented Jul 17, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@d7e0013). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #129   +/-   ##
=========================================
  Coverage          ?   55.65%           
  Complexity        ?      209           
=========================================
  Files             ?       90           
  Lines             ?     1335           
  Branches          ?      174           
=========================================
  Hits              ?      743           
  Misses            ?      518           
  Partials          ?       74
Impacted Files Coverage Δ Complexity Δ
...ory/src/main/kotlin/kategory/instances/Traverse.kt 0% <0%> (ø) 0 <0> (?)
...y/src/main/kotlin/kategory/typeclasses/Foldable.kt 0% <0%> (ø) 0 <0> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7e0013...0ab215c. Read the comment docs.

@JorgeCastilloPrz
Copy link
Member

Why moving an instance related function to be an extension function makes any difference in terms of usage? I guess it shouldn't, since that's the idea of extension functions, so they are used exactly the same way a function from an instance of the same type would be used, isn't it?

So how is this change helping us to use the global instance helper classes over it? I would love a bit deeper description about what we are trying to do here, otherwise It's not possible to get reviewed.

Copy link
Member

@raulraja raulraja left a comment

Choose a reason for hiding this comment

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

👏

@pakoito
Copy link
Member Author

pakoito commented Jul 17, 2017

@JorgeCastilloPrz We have a series of lookup methods for the different typeclasses, loosely based off implicits on Scala. To do the lookup they use a map from runtime Class objects to the instances.

When using generics, accessing the types of the parameters functions are generalized for isn't possible due to JVM limitations: type erasure. At runtime all List<T> are simple List, same for HK, Monad, etc... If you try to do T::class in Java it'll tell you it doesn't understand what T is, as it's not an object but a compile-time tag that turns into Object at runtime.

Kotlin adds a new tool to the JVM arsenal: reified generics. A reified generic is an indication that one or more of the parameters has to also be captured as a runtime Class value. This allows us to call the lookup functions with them. Another example of other functions that only work with reified generics are the arrayOf() ones. To enable reified generics it is required that the function is inlined.

For a function to be inlined there are some requirements: in this case not being virtual (cannot be possibly overridden). Functions in an interface are virtual by default, as per interface semantics (#128 (comment)), so our only option is making them into extension functions. They look the same, perform better, disable weird OOP overrides, don't have any co/contravariance semantics, and allow lookup. Extension functions are syntactic sugar for the FP way of having everything as a function whose first parameter is the context where they're applied to.

Copy link
Member

@JorgeCastilloPrz JorgeCastilloPrz left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation.

@pakoito pakoito merged commit 501d6af into master Jul 17, 2017
@pakoito pakoito deleted the paco-foldabletweaks branch July 17, 2017 10:13
@wiyarmir wiyarmir mentioned this pull request Jul 17, 2017
rachelcarmena pushed a commit that referenced this pull request Feb 24, 2021
* Implement `filterIsInstance` and `traverseFilterIsInstance`

* Fix after merge

* Fix :arrow-core-data:ktlintMainSourceSetCheck

* Fix Ank problem

Co-authored-by: Alberto Ballano <aballano@users.noreply.github.com>
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