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

Create StateT type #25

Merged
merged 13 commits into from
Jun 13, 2017
Merged

Create StateT type #25

merged 13 commits into from
Jun 13, 2017

Conversation

arturogutierrez
Copy link
Member

@arturogutierrez arturogutierrez commented Mar 24, 2017

Fixes #14

Super basic example of State Monad:

// State for a random 6-dice roll
val dice = State<Random, Int> { r -> Pair(r, r.nextInt(6) + 1) }
// Applying a +3 modifier to the roll
val modDice = dice.map { n -> (n + 3).coerceAtMost(6) } // return a new State<Random, Int> generating only 4-5-6 possible dice values
// Roll example
val d1 = modDice.runA(Random())

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.

Excellent, let's replace eval with run. And let's provide runA, runS and run to get just A, just the State or both accordingly

Copy link
Contributor

@JMPergar JMPergar left a comment

Choose a reason for hiding this comment

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

Apply the changes requested by @raulraja and you will have my approved too

@codecov
Copy link

codecov bot commented Mar 31, 2017

Codecov Report

Merging #25 into master will decrease coverage by 1.36%.
The diff coverage is 26.78%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #25      +/-   ##
============================================
- Coverage     54.82%   53.46%   -1.37%     
- Complexity      177      187      +10     
============================================
  Files            77       80       +3     
  Lines          1098     1154      +56     
  Branches        169      169              
============================================
+ Hits            602      617      +15     
- Misses          433      474      +41     
  Partials         63       63
Impacted Files Coverage Δ Complexity Δ
katz/src/main/kotlin/katz/instances/StateTMonad.kt 0% <0%> (ø) 0 <0> (?)
katz/src/main/kotlin/katz/data/State.kt 100% <100%> (ø) 2 <2> (?)
katz/src/main/kotlin/katz/data/StateT.kt 32.5% <32.5%> (ø) 8 <8> (?)

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 7f10e83...05b0a93. Read the comment docs.

@arturogutierrez
Copy link
Member Author

PR updated! :)

Copy link
Member

@ffgiraldez ffgiraldez left a comment

Choose a reason for hiding this comment

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

Update State class to HKT and use Tupled class instead of Pair

* to returning the result of type A, the function
* returns a new S value, which is the updated state.
*/
class State<S, out A>(val runF: (S) -> Pair<S, A>) {
Copy link
Member

Choose a reason for hiding this comment

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

please use Tuple2 instead of Pair

Copy link
Member

Choose a reason for hiding this comment

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

make it implement HK2<State.F,S,A>

Copy link
Member Author

Choose a reason for hiding this comment

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

AFAIK, StateT is who has to support HKT, right? This State is using Identity as Monad Environment, isn't?

Copy link
Member Author

Choose a reason for hiding this comment

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

About Tuple2, I will change it :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Raul's comment was very clear 😄
I will try to implement StateT 💪

@raulraja
Copy link
Member

Now that we have Id instead of State this type should be StateT. StateT has three type args.
F, S and A where F is some other monad. once StateT is implemented you can derive State by
type State[S, A] = StateT[Id, S, A].
StateT allows us to use state in other context beside a plain environment such as Id for example Future.

fun runS(s: S): S = run(s).a

fun <B> map(f: (A) -> B): State<S, B> {
return State { s1 ->
Copy link
Member

Choose a reason for hiding this comment

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

you can remove this return by using fun <B> map(f: (A) -> B) = State<S, B> { ... } syntax

@raulraja
Copy link
Member

raulraja commented Apr 2, 2017

@arturogutierrez take a look at how Reader integrates with Kleisli now in master, you are gonna have to do something similar for StateT and State.

@ffgiraldez ffgiraldez changed the title Create State type Create StateT type Apr 23, 2017
@ffgiraldez
Copy link
Member

I will gonna update this PR soon

Copy link
Member

@pakoito pakoito left a comment

Choose a reason for hiding this comment

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

Can you please add the Monad instances? It should be just evaluating the parameter and calling the methods in the object.

Copy link
Member

@pakoito pakoito left a comment

Choose a reason for hiding this comment

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

LGTM for shipping. I have never had to overwrite map2 and map2Eval, is there a reason for this? It'd be great to have tests for them.

@ffgiraldez
Copy link
Member

i see that map2 and map2Eval was defined here in cats

Now that @raulraja are pushing laws i will not do more test on typeclasses and focus on help him to finish it.

@pakoito pakoito merged commit 60ccc36 into master Jun 13, 2017
@pakoito pakoito deleted the create-state-type branch June 13, 2017 09:26
FPerezP pushed a commit to FPerezP/kategory that referenced this pull request Dec 12, 2017
rachelcarmena added a commit that referenced this pull request Feb 24, 2021
rachelcarmena added a commit that referenced this pull request Feb 24, 2021
rachelcarmena added a commit that referenced this pull request Feb 24, 2021
* 0.11.0/bio (#1975)

* BIO - Part 1 (#1851)

* Add E param

* Fix imports & types

* Add RaiseError case

* Refactor IO name

* [BIO]: include `E` into IOFrame & IORunLoop (#1859)

* Rewrite IOFrame

* Update IORunLoop & fix resulting combinators

* Move public code to IO.kt

* Refactor Right to Success

* IO: IOConnection & IOBracket (#1870)

* Update ForwardCancelable

* Update IOBracket

* Remove KindConnection and refactor into IOConnection

* IO Concurrency combinators & Green Build (#1892)

* BIO - Part 1 (#1851)

* Add E param

* Fix imports & types

* Add RaiseError case

* Refactor IO name

* [BIO]: include `E` into IOFrame & IORunLoop (#1859)

* Rewrite IOFrame

* Update IORunLoop & fix resulting combinators

* Move public code to IO.kt

* Refactor Right to Success

* IO: IOConnection & IOBracket (#1870)

* Update ForwardCancelable

* Update IOBracket

* Remove KindConnection and refactor into IOConnection

* Rewrite UnsafePromise

* Rewrite ParMap2

* Update IOParMap3

* Update IORacePair

* Update IORaceTriple

* Fix tests

* Fix KIO benchmarks

* Fix imports Ank

* Fix IOExample

* Fix unused imports

* Fix imports benchmarks

* Fix arrow-integrations-retrofit-adapter

* Fix FpToTheMax & typeclasses example

* KtLintFormat

* Fix IOTest

* Fix more tests

* Fix merge conflicts

* KtLintFormat

* Fix MaybeK bracket and use it for QueueTest

* Throw proper exception in impossible cases

* Revert removing runAsync

* Fix build

* KtLintFormat

* Update version

* After releasing 0.10.5: just considering one wip version (#26)

Co-authored-by: Simon Vergauwen <[email protected]>
Co-authored-by: Simon Vergauwen <[email protected]>
Co-authored-by: raulraja <[email protected]>
rachelcarmena added a commit that referenced this pull request Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants