custom Result #1

krzykrucz · 2019-11-30T19:01:48Z

No description provided.

result/LICENSE.md

jakubiec · 2019-11-30T20:36:57Z

result-rxjava/src/test/kotlin/com/virtuslab/basetypes/result/rxjava/SingleResultKtTest.kt

        }

-        "should handle exception when mapping result success" {
+        "should propagate exception when mapping result success" {
            val monoResult: SingleResult<String, SomeFailure> = "Some value".justSingleResult()
            val runtimeException = RuntimeException()

            monoResult.flatMapResult { if (true) throw runtimeException else Result.success("Some other value") }


if (true) throw runtimeException else Result.success("Some other value") this condition is always true. Why it was done this way? Btw. it's also duplicated in this test

if(true) is here because I want an exception to be always thrown

else branch is so that the code compiles

jakubiec · 2019-11-30T20:42:21Z

result-reactor/src/test/kotlin/com/virtuslab/basetypes/result/reactor/MonoResultKtTest.kt

@@ -34,8 +33,7 @@ internal class MonoResultKtTest : StringSpec() {

            monoResult.flatMapResult { if (true) throw runtimeException else Result.success("Some other value") }


same comment here about always true condition

jakubiec · 2019-11-30T20:47:58Z

result/src/test/kotlin/com/github/kittinunf/result/ResultTests.kt

@@ -0,0 +1,414 @@
+//package com.github.kittinunf.result


why this test is commented out?

jakubiec · 2019-11-30T20:48:15Z

result/src/test/kotlin/com/github/kittinunf/result/ValidationTests.kt

+//import org.junit.Test
+//import org.hamcrest.CoreMatchers.`is` as isEqualTo
+//
+//class ValidationTests {


why this test is commented out?

jakubiec · 2019-11-30T20:51:26Z

I spotted something interesting in main build.gradle.kts, namely below snippet:

    configure<BintrayExtension> {
        user = findProperty("BINTRAY_USER") as? String
        key = findProperty("BINTRAY_KEY") as? String
        setPublications(project.name)
        publish = true
        pkg.apply {
            repo = "maven"
            name = "base-types-kt"
            desc = "The modelling for success/failure of operations in Kotlin"
            userOrg = "kittinunf"
            websiteUrl = "https://github.com/krzykrucz/base-types-kt"
            vcsUrl = "https://github.com/krzykrucz/base-types-kt"
            setLicenses("MIT")
            version.apply {
                name = artifactPublish
            }
        }
    }

it looks like a mix of your own branch with MIT license but referes to kittinunf userOrg. Shouldn't it refer to VirtusLab repo and organisation?

jakubiec · 2019-11-30T20:52:36Z

as I understand, it's just a copy from some point in time of Result and we don't want to just fork it and eventually introduce new changes from the original location?

result/src/main/kotlin/com/github/kittinunf/result/Result.kt

jakubiec · 2019-11-30T20:56:54Z

result/src/main/kotlin/com/github/kittinunf/result/Result.kt

+
+fun <V : Any, E : Exception> Result<V, E>.any(predicate: (V) -> Boolean): Boolean = try {
+    when (this) {
+        is Result.Success -> predicate(value)


here also we may spot an exception

ah...nvm, its covered with try

but that's the case, some trys were removed here?

jakubiec

Looks nicey but there are some comments to discuss

FatCaterpillar · 2019-12-01T13:12:14Z

result/src/main/kotlin/com/github/kittinunf/result/Result.kt

+fun <V : Any, U : Any> Result<V, *>.fanout(other: () -> Result<U, *>): Result<Pair<V, U>, *> =
+        flatMap { outer -> other().map { outer to it } }
+
+fun <V : Any, E : Exception> List<Result<V, E>>.lift(): Result<List<V>, E> = fold(Result.success(mutableListOf<V>()) as Result<MutableList<V>, E>) { acc, result ->


imo it should be named sequence as in other FP libraries, I'd expect lift to only wrap some value in container

FatCaterpillar · 2019-12-01T13:15:30Z

result/src/main/kotlin/com/github/kittinunf/result/Result.kt

+    false
+}
+
+fun <V : Any, U : Any> Result<V, *>.fanout(other: () -> Result<U, *>): Result<Pair<V, U>, *> =


I'd name it zip

codecov-io · 2019-12-07T17:51:17Z

Codecov Report

Merging #1 into master will decrease coverage by 0.64%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master       #1      +/-   ##
============================================
- Coverage     85.31%   84.67%   -0.65%     
  Complexity       18       18              
============================================
  Files            10       10              
  Lines           143      137       -6     
  Branches         14       14              
============================================
- Hits            122      116       -6     
  Misses           19       19              
  Partials          2        2

Impacted Files	Coverage Δ	Complexity Δ
...m/virtuslab/basetypes/result/reactor/MonoResult.kt	`91.66% <100%> (-0.93%)`	`0 <0> (ø)`
.../virtuslab/basetypes/result/rxjava/SingleResult.kt	`82.14% <100%> (-1.73%)`	`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 c7af734...f3833a2. Read the comment docs.

krzykrucz added 2 commits November 30, 2019 19:53

propagating Result exceptions

599fb23

propagating Result exceptions in result-reactor

dae5d18

jakubiec reviewed Nov 30, 2019

View reviewed changes

result/LICENSE.md Show resolved Hide resolved

jakubiec reviewed Nov 30, 2019

View reviewed changes

result/src/main/kotlin/com/github/kittinunf/result/Result.kt Outdated Show resolved Hide resolved

jakubiec reviewed Nov 30, 2019

View reviewed changes

result/src/main/kotlin/com/github/kittinunf/result/Result.kt Outdated Show resolved Hide resolved

jakubiec reviewed Nov 30, 2019

View reviewed changes

FatCaterpillar reviewed Dec 1, 2019

View reviewed changes

krzykrucz added 2 commits December 7, 2019 18:45

result creation functions

695e495

result creation functions

f3833a2

Code Review minor corrections

b998365

krzykrucz merged commit 91614c0 into master Dec 7, 2019

krzykrucz deleted the result branch March 12, 2020 15:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

custom Result #1

custom Result #1

krzykrucz commented Nov 30, 2019

jakubiec Nov 30, 2019

krzykrucz Dec 7, 2019

jakubiec Nov 30, 2019

jakubiec Nov 30, 2019

jakubiec Nov 30, 2019

jakubiec commented Nov 30, 2019

jakubiec commented Nov 30, 2019

jakubiec Nov 30, 2019

jakubiec Nov 30, 2019

jakubiec Nov 30, 2019

jakubiec left a comment

FatCaterpillar Dec 1, 2019

FatCaterpillar Dec 1, 2019

codecov-io commented Dec 7, 2019 •

edited

Loading

		@@ -34,8 +33,7 @@ internal class MonoResultKtTest : StringSpec() {

		monoResult.flatMapResult { if (true) throw runtimeException else Result.success("Some other value") }

custom Result #1

custom Result #1

Conversation

krzykrucz commented Nov 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakubiec commented Nov 30, 2019

jakubiec commented Nov 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakubiec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Dec 7, 2019 • edited Loading

Codecov Report

codecov-io commented Dec 7, 2019 •

edited

Loading