Fix regression in lifted semigroup sum. Move lifted definition in Monoid.

jbgi · jbgi · commit cf1188acafd6 · 2018-10-07T13:56:47.000+02:00
diff --git a/core/src/main/java/fj/Monoid.java b/core/src/main/java/fj/Monoid.java
@@ -17,6 +17,7 @@
 import static fj.data.List.nil;
 import static fj.data.Natural.natural;
 import static fj.data.Option.none;
+import static fj.data.Option.some;
 import static fj.data.Stream.iterableStream;
 
 import java.math.BigInteger;
@@ -867,6 +868,36 @@ public List<A> sum(F0<Stream<List<A>>> as) {
     });
   }
 
+  /**
+   * Lift a {@code Semigroup<A>} for A to a {@code Monoid<Option<A>>}, using Option.none() as zero.
+   *
+   * @return A monoid for option.
+   */
+  public static <A> Monoid<Option<A>> optionMonoid(Semigroup<A> aSemigroup) {
+    return monoidDef(new Monoid.Definition<Option<A>>() {
+      @Override
+      public Option<A> empty() {
+        return none();
+      }
+
+      @Override
+      public Option<A> append(Option<A> a1, Option<A> a2) {
+        return a1.liftM2(a2, aSemigroup::sum).orElse(a1).orElse(a2);
+      }
+
+      @Override
+      public Option<A> multiply(int n, Option<A> oa) {
+        return n > 0 ? oa.map(a -> aSemigroup.multiply1p(n - 1, a)) : none();
+      }
+
+      @Override
+      public Option<A> sum(F0<Stream<Option<A>>> oas) {
+        Stream<A> as = oas.f().bind(Option::toStream);
+        return as.uncons(none(), h -> tail ->  some(aSemigroup.sumStream(h, tail::_1)));
+      }
+    });
+  }
+
   /**
    * A monoid for options.
    * @deprecated since 4.7. Use {@link #firstOptionMonoid()}.
diff --git a/core/src/main/java/fj/Semigroup.java b/core/src/main/java/fj/Semigroup.java
@@ -172,29 +172,7 @@ public Semigroup<A> dual() {
    * Lifts the semigroup to obtain a trivial monoid.
    */
   public Monoid<Option<A>> lift() {
-    Definition<A> def = this.def;
-    return monoidDef(new Monoid.Definition<Option<A>>() {
-      @Override
-      public Option<A> empty() {
-        return none();
-      }
-
-      @Override
-      public Option<A> append(Option<A> a1, Option<A> a2) {
-        return a1.liftM2(a1, def::append).orElse(a1).orElse(a2);
-      }
-
-      @Override
-      public Option<A> multiply(int n, Option<A> oa) {
-        return n > 0 ? oa.map(a -> def.multiply1p(n - 1, a)) : none();
-      }
-
-      @Override
-      public Option<A> sum(F0<Stream<Option<A>>> oas) {
-        Stream<A> as = oas.f().bind(Option::toStream);
-        return as.uncons(none(), h -> tail ->  some(def.sum(h, tail::_1)));
-      }
-    });
+    return Monoid.optionMonoid(this);
   }
 
   /**
diff --git a/core/src/test/java/fj/MonoidTest.java b/core/src/test/java/fj/MonoidTest.java
@@ -0,0 +1,19 @@
+package fj;
+
+import fj.data.Option;
+import fj.data.Stream;
+import org.junit.Test;
+
+import static fj.data.Option.some;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertThat;
+
+public class MonoidTest {
+
+  @Test
+  public void lifted_sum_of_two_numbers() {
+    Monoid<Option<Integer>> optionMonoid = Semigroup.intAdditionSemigroup.lift();
+    assertThat(optionMonoid.sum(some(3), some(5)), is(some(8)));
+    assertThat(optionMonoid.sumLeft(Stream.arrayStream(some(3), some(5))), is(some(8)));
+  }
+}
