ReaderT#fmap and StateT#fmap don't unnecessarily rely on pure

jnape · jnape · commit d3a049c8df1b · 2020-11-11T14:03:50.000-06:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
 
 ### Changed
 - `IterateT#unfold` now only computes a single `Pure` for the given input
+- `ReaderT#fmap` and `StateT#fmap` avoid unnecessary calls to `pure`
 
 ### Added
 - `$`, function application represented as a higher-order `Fn2`
diff --git a/src/main/java/com/jnape/palatable/lambda/monad/transformer/builtin/ReaderT.java b/src/main/java/com/jnape/palatable/lambda/monad/transformer/builtin/ReaderT.java
@@ -120,7 +120,7 @@ public <B> ReaderT<R, M, B> pure(B b) {
      */
     @Override
     public <B> ReaderT<R, M, B> fmap(Fn1<? super A, ? extends B> fn) {
-        return MonadT.super.<B>fmap(fn).coerce();
+        return readerT(r -> runReaderT(r).fmap(fn));
     }
 
     /**
diff --git a/src/main/java/com/jnape/palatable/lambda/monad/transformer/builtin/StateT.java b/src/main/java/com/jnape/palatable/lambda/monad/transformer/builtin/StateT.java
@@ -144,7 +144,7 @@ public <B> StateT<S, M, B> pure(B b) {
      */
     @Override
     public <B> StateT<S, M, B> fmap(Fn1<? super A, ? extends B> fn) {
-        return MonadT.super.<B>fmap(fn).coerce();
+        return stateT(s -> runStateT(s).fmap(t -> t.biMapL(fn)));
     }
 
     /**
@@ -292,7 +292,7 @@ public static <S, M extends MonadRec<?, M>, A> StateT<S, M, A> stateT(
     public static <S, M extends MonadRec<?, M>> Pure<StateT<S, M, ?>> pureStateT(Pure<M> pureM) {
         return new Pure<StateT<S, M, ?>>() {
             @Override
-            public <A> StateT<S, M, A> checkedApply(A a) throws Throwable {
+            public <A> StateT<S, M, A> checkedApply(A a) {
                 return stateT(pureM.<A, MonadRec<A, M>>apply(a));
             }
         };
diff --git a/src/test/java/com/jnape/palatable/lambda/monad/transformer/builtin/ReaderTTest.java b/src/test/java/com/jnape/palatable/lambda/monad/transformer/builtin/ReaderTTest.java
@@ -2,6 +2,7 @@
 
 import com.jnape.palatable.lambda.adt.Maybe;
 import com.jnape.palatable.lambda.adt.Unit;
+import com.jnape.palatable.lambda.functions.Fn1;
 import com.jnape.palatable.lambda.functor.builtin.Identity;
 import com.jnape.palatable.lambda.io.IO;
 import com.jnape.palatable.traitor.annotations.TestTraits;
@@ -12,6 +13,7 @@
 
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import static com.jnape.palatable.lambda.adt.Maybe.just;
 import static com.jnape.palatable.lambda.adt.Unit.UNIT;
@@ -82,4 +84,17 @@ public void composedZip() {
                 .unsafePerformAsyncIO(Executors.newFixedThreadPool(2))
                 .join();
     }
+
+    @Test
+    public void fmapInteractions() {
+        AtomicInteger invocations = new AtomicInteger(0);
+        ReaderT<Integer, Identity<?>, Integer> readerT = readerT(i -> {
+            invocations.incrementAndGet();
+            return new Identity<>(i);
+        });
+
+        Fn1<Integer, Integer> plusOne = x -> x + 1;
+        readerT.fmap(plusOne).fmap(plusOne).fmap(plusOne).runReaderT(0);
+        assertEquals(1, invocations.get());
+    }
 }
diff --git a/src/test/java/com/jnape/palatable/lambda/monad/transformer/builtin/StateTTest.java b/src/test/java/com/jnape/palatable/lambda/monad/transformer/builtin/StateTTest.java
@@ -16,15 +16,18 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import static com.jnape.palatable.lambda.adt.Maybe.just;
 import static com.jnape.palatable.lambda.adt.Unit.UNIT;
 import static com.jnape.palatable.lambda.adt.hlist.HList.tuple;
+import static com.jnape.palatable.lambda.functions.builtin.fn1.Id.id;
 import static com.jnape.palatable.lambda.functor.builtin.Identity.pureIdentity;
 import static com.jnape.palatable.lambda.optics.functions.Set.set;
 import static com.jnape.palatable.lambda.optics.lenses.ListLens.elementAt;
 import static java.util.Arrays.asList;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
 import static testsupport.matchers.StateTMatcher.whenEvaluated;
 import static testsupport.matchers.StateTMatcher.whenExecuted;
 import static testsupport.matchers.StateTMatcher.whenRun;
@@ -119,4 +122,18 @@ public void staticLift() {
         assertThat(StateT.<String>liftStateT().apply(new Identity<>(1)),
                    whenRun("foo", new Identity<>(tuple(1, "foo"))));
     }
+
+    @Test
+    public void fmapInteractions() {
+        AtomicInteger invocations = new AtomicInteger(0);
+        StateT.<Integer, Identity<?>, Integer>gets(x -> {
+            invocations.incrementAndGet();
+            return new Identity<>(x);
+        })
+                .fmap(id())
+                .fmap(id())
+                .fmap(id())
+                .<Identity<Tuple2<Integer, Integer>>>runStateT(0);
+        assertEquals(1, invocations.get());
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ public <B> ReaderT<R, M, B> pure(B b) {`
`120`	`120`	`*/`
`121`	`121`	`@Override`
`122`	`122`	`public <B> ReaderT<R, M, B> fmap(Fn1<? super A, ? extends B> fn) {`
`123`		`- return MonadT.super.<B>fmap(fn).coerce();`
	`123`	`+ return readerT(r -> runReaderT(r).fmap(fn));`
`124`	`124`	`}`
`125`	`125`
`126`	`126`	`/**`