-
Notifications
You must be signed in to change notification settings - Fork 454
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
PartialFunction + match(case ..) statements #143
Conversation
Codecov Report
@@ Coverage Diff @@
## master #143 +/- ##
=========================================
Coverage ? 55.46%
Complexity ? 170
=========================================
Files ? 71
Lines ? 1509
Branches ? 181
=========================================
Hits ? 837
Misses ? 593
Partials ? 79
Continue to review full report at Codecov.
|
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ |
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 believe we don't use these per file anymore.
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.
just a like question, but just to clarify me.
package kategory | ||
|
||
@Suppress("UNCHECKED_CAST") | ||
fun <A: Any, B> Iterable<A>.collect(vararg cases: PartialFunction<*, B>): List<B> { |
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.
it's ok, handle Iterables and return a List? It's mandatory that collect()
return a List
or it could return something more generic like Iterable
or Collection
?
result shouldBe 1 | ||
} | ||
|
||
"match nested statements" { |
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.
Travis point that this function does not pass the test PartialFunctionTests.match nested statements FAILED java.lang.ClassCastException
could you take a look @pakoito?
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 proposed this test because I suspected it wouldn't pass. @raulraja is on it!
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.
hahaha, Ok good to know.
…n control erasure type casts accordingly.
Try({ it as A }).fold({ false }, { true }) | ||
} | ||
|
||
infix fun <A, B> ((A) -> Boolean).then(f: (A) -> B): Tuple2<(A) -> Boolean, (A) -> B> = Tuple2(this, f) |
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 believe the functions then
, typeof
, case
should be scoped to the class match
or PartialFuncion
so they don't scape the DSL.
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.
The issue is that for those to be scoped they would need to be placed inside the companion object and would have to be individually imported:
import kategory.match.dsl.case
import kategory.match.dsl.typeOf
...
I have never been able to import members of companions with wildcards in Kotlin and it feels to me that those extra imports are an unnecessary burden to the user. Also this will be the first time we require additional imports beside kategory.*
are we sure we want to do that?
Alternatively I can create a new kategory.cases
package for these functions but also the users would have to explicitly import import kategory.cases.*
Thoughts?
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 see. Yes, let's discuss whether this is part of the main package or an extension some other time. It's time!
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.
Just for this feature many people are going to consider the library :D
From the gradle docs: The compile configuration still exists but should not be used as it will not offer the guarantees that the api and implementation configurations provide. https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_separation
The following PR brings supports to PartialFunctions for Kotlin through Kategory.
It includes syntax for
match
andcase
and extensions forIterable.collect
so they work like in Scala.