Factor: a cache hit returns what the first call returned

symja_android_library/matheclipse-core/src/main/java/org/matheclipse/core/builtin/Algebra.java

@@ -1643,8 +1643,12 @@ public IExpr evaluate(final IAST ast, final int argSize, final IExpr[] options,
       }
       if (modulus.isZero() && extension == S.None && gaussianIntegers.isFalse()) {
         IExpr factorization = AlgebraUtil.factor(ast, arg1, eVar, false, true, true, trig, engine);
-        return factorization.isPresent() ? AlgebraUtil.factorSigns(factorization, engine)
-            : factorization;
+        if (factorization.isPresent()) {
+          factorization = AlgebraUtil.factorSigns(factorization, engine);
+          // cache what is returned, so that the cache hit above gives the same form
+          engine.putCache(ast, factorization);
+        }
+        return factorization;
       }
       if (!modulus.isZero()) {
         return factorModulus(S.Factor, arg1, eVar, false, modulus, engine);

symja_android_library/matheclipse-core/src/main/java/org/matheclipse/core/eval/AlgebraUtil.java

@@ -1583,6 +1583,10 @@ private static IExpr[] extractFractionalPower(IExpr expr) {
     return null;
   }
 
+  /**
+   * Writes nothing to the engine cache: the built-in <code>Factor</code> caches the result it
+   * returns itself, and the other callers factor with other settings under the same key.
+   */
   public static IExpr factor(IAST ast, IExpr arg1, VariablesSet eVar, boolean squareFree,
       boolean withHomogenization, boolean togetherMode, boolean trig, EvalEngine engine) {
     IExpr expr = arg1;
@@ -1599,11 +1603,8 @@ public static IExpr factor(IAST ast, IExpr arg1, VariablesSet eVar, boolean squa
             IExpr denominator = factorExpr(F.Factor(fractionParts[1]), fractionParts[1], eVar,
                 squareFree, withHomogenization, trig, engine);
             if (numerator.isPresent() && denominator.isPresent()) {
-              IExpr temp = engine.evaluate(F.Divide(numerator, denominator));
-              engine.putCache(ast, temp);
-              return temp;
+              return engine.evaluate(F.Divide(numerator, denominator));
             }
-            engine.putCache(ast, F.NIL);
           } catch (JASConversionException e) {
             //
           }
@@ -1613,7 +1614,6 @@ public static IExpr factor(IAST ast, IExpr arg1, VariablesSet eVar, boolean squa
     }
     try {
       IExpr temp = factorExpr(ast, expr, eVar, squareFree, withHomogenization, trig, engine);
-      engine.putCache(ast, temp);
       if (temp.isPresent()) {
         return temp;
       }

symja_android_library/matheclipse-core/src/test/java/org/matheclipse/core/system/AlgebraTest.java

@@ -717,4 +717,27 @@ public void testFactorIssue1557() {
         "(1+x)*z");
   }
 
+  /**
+   * A cache hit gives what the first Factor of the same argument returned, and a Simplify which
+   * factors the argument internally does not change what a later Factor returns.
+   */
+  @Test
+  public void testFactorIssue1570() {
+    check("{Factor(6/(2-x)), Factor(6/(2-x))}", //
+        "{-6/(-2+x),-6/(-2+x)}");
+    check("{Factor((-a+b)^3), Factor((-a+b)^3)}", //
+        "{-(a-b)^3,-(a-b)^3}");
+    check("{Factor((x+2)/(x-5)+3==6/(2-x)), Factor((x+2)/(x-5)+3==6/(2-x))}", //
+        "{(-13+4*x)/(-5+x)==-6/(-2+x),(-13+4*x)/(-5+x)==-6/(-2+x)}");
+    // TrigSimplifyFu, which Simplify and FullSimplify run, factors the argument first
+    check("Last({TrigSimplifyFu(6/(2-x)), Factor(6/(2-x))})", //
+        "-6/(-2+x)");
+    check("Last({Simplify(Cos(y)^2/(2-x)), Factor(Cos(y)^2/(2-x))})", //
+        "-Cos(y)^2/(-2+x)");
+    check("Last({FullSimplify((-a+b)^3*Cos(y)), Factor((-a+b)^3*Cos(y))})", //
+        "-(a-b)^3*Cos(y)");
+    check("Last({Simplify(6/(2-x)+Cos(y)), Factor(6/(2-x)+Cos(y))})", //
+        "(-6-2*Cos(y)+x*Cos(y))/(-2+x)");
+  }
+
 }