uncurry store update function and drop Option wrapper around effects

[buntec]

No description provided.

[johnynek]

You could avoid this I think since case matches at the top level of a function are allowed (due to the questionable PartialFunction Function subclass relationship)

[johnynek]

Can omit this I think.

[johnynek]

again, could be:

_ => 
  {
     case (SetFruit(fruit), state) => state.copy(fruit = fruit) -> F.unit
     case (Inc, state) => state.copy(counter = state.counter + 1) -> F.unit
}

I think.

[johnynek]

I wonder if we want something like this:

def simple[F[_]: Concurrent, State, Action](init: State)(update: (Action, State) => State): Store[F, State, Action] = {
  val unit = Concurrent[F].unit
  apply[F, State, Action](init) { _ => { (a, s) => (update(a, s), unit) } }
}

Then I think many simple apps can become something like:

store <- ff4s.Store.simple[F, State, Action](State())) {
  case (Action.Toggle(), state) => state.copy(toggle = !state.toggle)
}

or similar.

Also, adding a method def flipToggle: State to your case class State this example becomes even cleaner:

store <- ff4s.Store.simple[F, State, Action](State())) {
  case (Action.Toggle(), state) => state.flipToggle
}

[johnynek]

I would make one call to unit and internally cache. For internal code of a library, I bias towards avoiding allocations. In fact here I would write:

.flatMap { fu =>
  if (fu == unit) unit
  else supervisor.supervise(fu)
}

so the additional cost in the case of non-unit effect is 1 additional .equals and 1 additional branch (which possibly a branch predictor would basically wind up hiding the cost of.

[buntec]

I'm not sure I understand: if we know that unit is a val for cats.effect.IO (and presumably any other reasonable implementation), then where is the allocation? Also, I think I prefer keeping unlessA b/c it makes the intent clear and the cost is just one additional function call (hopefully inlined by the JIT).

[johnynek]

many (most?) of the examples are of the simple kind I mentioned above. I think having a helper on Store to support this would be really nice.

[johnynek]

This looks really great to me.

[johnynek]

The second argument is a call by name param which requires an allocation but it's probably not a big deal if you much prefer it.

[johnynek]

This is great. I like the name too. Maybe it would be a a good idea to use it in one or two of your existing examples?

[buntec]

Thanks so much for this!