[poincare] Change design of immediateBeautify

Change-Id: Ifb30b89b30af1df7eca855cc2cea2da75206fc96

poincare/include/poincare/addition.h

@@ -37,7 +37,7 @@ private:
     return LayoutEngine::createInfixLayout(this, floatDisplayMode, complexFormat, "+");
   }
   /* Simplification */
-  void immediateBeautify() override;
+  Expression * immediateBeautify() override;
   void factorizeChildren(Expression * e1, Expression * e2);
   static const Rational RationalFactor(Expression * e);
   static bool TermsHaveIdenticalNonRationalFactors(const Expression * e1, const Expression * e2);

poincare/include/poincare/dynamic_hierarchy.h

@@ -24,7 +24,7 @@ public:
   void addOperandAtIndex(Expression * operand, int index);
   void mergeOperands(DynamicHierarchy * d);
   void sortChildren();
-  void squashUnaryHierarchy();
+  Expression * squashUnaryHierarchy();
 protected:
   bool deleteUselessOperand(int index);
 private:

poincare/include/poincare/expression.h

@@ -132,9 +132,10 @@ public:
   ExpressionLayout * createLayout(FloatDisplayMode floatDisplayMode = FloatDisplayMode::Default, ComplexFormat complexFormat = ComplexFormat::Default) const; // Returned object must be deleted
 
   /* Simplification */
-  static void simplifyAndBeautify(Expression ** e);
+  static void simplifyAndBeautify(Expression ** expressionAddress);
   // TODO: should be virtual pure
   virtual void immediateSimplify() {};// = 0;
+  virtual Expression * immediateBeautify() { return this; };
 
   /* Evaluation Engine
    * The function evaluate creates a new expression and thus mallocs memory.
@@ -161,8 +162,8 @@ private:
   virtual Evaluation<double> * privateEvaluate(DoublePrecision p, Context& context, AngleUnit angleUnit) const = 0;
   /* Simplification */
   void simplify();
-  void beautify();
-  virtual void immediateBeautify() {};
+  // beautify cannot be dynamic as it changes the expression and THEN its new children
+  static void beautify(Expression ** expressionAddress);
   /* Sorting */
   virtual int compareToGreaterTypeExpression(const Expression * e) const {
     return -1;

poincare/include/poincare/multiplication.h

@@ -46,9 +46,9 @@ private:
   static bool TermHasRationalExponent(const Expression * e);
   static const Expression * CreateExponent(Expression * e);
   bool isUselessOperand(const Rational * r) override;
-  // Warning: mergeNegativePower return always a multiplication: *(b^-1,c^-1) -> *((bc)^-1)
-  void immediateBeautify() override;
-  void mergeNegativePower();
+  // Warning: mergeNegativePower not always returns  a multiplication: *(b^-1,c^-1) -> (bc)^-1
+  Expression * immediateBeautify() override;
+  Expression * mergeNegativePower();
 };
 
 }

poincare/src/addition.cpp

@@ -94,7 +94,7 @@ bool Addition::TermsHaveIdenticalNonRationalFactors(const Expression * e1, const
   return (f1->compareTo(f2) == 0);
 }
 
-void Addition::immediateBeautify() {
+Expression * Addition::immediateBeautify() {
   int index = 0;
   while (index < numberOfOperands()) {
     // a+(-1)*b+... -> a-b+...
@@ -116,6 +116,7 @@ void Addition::immediateBeautify() {
     }
     index++;
   }
+  return this;
 }
 
 

poincare/src/dynamic_hierarchy.cpp

@@ -151,11 +151,14 @@ void DynamicHierarchy::sortChildren() {
   }
 }
 
-void DynamicHierarchy::squashUnaryHierarchy() {
+Expression * DynamicHierarchy::squashUnaryHierarchy() {
   assert(parent() != nullptr);
   if (numberOfOperands() == 1) {
-    replaceWith(const_cast<Expression *>(operand(0)), true);
+    Expression * o = const_cast<Expression *>(operand(0));
+    replaceWith(o, true);
+    return o;
   }
+  return this;
 }
 
 bool DynamicHierarchy::deleteUselessOperand(int index) {

poincare/src/expression.cpp

@@ -89,11 +89,12 @@ public:
   }
 };
 
-void Expression::simplifyAndBeautify(Expression ** e) {
-  SimplificationRoot root(*e);
+void Expression::simplifyAndBeautify(Expression ** expressionAddress) {
+  SimplificationRoot root(*expressionAddress);
   root.simplify();
-  root.beautify();
-  *e = (Expression *)(root.operand(0));
+  Expression * e = (Expression *)root.operand(0);
+  beautify(&e);
+  *expressionAddress = (Expression *)(root.operand(0));
 }
 
 void Expression::simplify() {
@@ -103,11 +104,12 @@ void Expression::simplify() {
   immediateSimplify();
 }
 
-void Expression::beautify() {
-  for (int i = 0; i < numberOfOperands(); i++) {
-    ((Expression *)operand(i))->beautify();
+void Expression::beautify(Expression ** expressionAddress) {
+  *expressionAddress = (*expressionAddress)->immediateBeautify();
+  for (int i = 0; i < (*expressionAddress)->numberOfOperands(); i++) {
+    Expression * e = (Expression *)(*expressionAddress)->operand(i);
+    beautify(&e);
   }
-  immediateBeautify();
 }
 
 bool Expression::hasAncestor(const Expression * e) const {

poincare/src/multiplication.cpp

@@ -132,8 +132,6 @@ void Multiplication::immediateSimplify() {
       i++;
     }
   }
-  // Merge negative power: a*b^-1*c^(-Pi)*d = a*(b*c^Pi)^-1
-  mergeNegativePower();
   squashUnaryHierarchy();
 }
 
@@ -208,7 +206,13 @@ bool Multiplication::isUselessOperand(const Rational * r) {
   return r->isOne();
 }
 
-void Multiplication::immediateBeautify() {
+Expression * Multiplication::immediateBeautify() {
+  // Merge negative power: a*b^-1*c^(-Pi)*d = a*(b*c^Pi)^-1
+  Expression * e = mergeNegativePower();
+  if (e->type() == Type::Power) {
+    return e->immediateBeautify();
+  }
+  assert(e == this);
   int index = 0;
   while (index < numberOfOperands()) {
     // a*b^(-1)*... -> a*.../b
@@ -240,13 +244,14 @@ void Multiplication::immediateBeautify() {
       }
       numeratorOperand->squashUnaryHierarchy();
       replaceWith(d, true);
-      return;
+      return d;
     }
     index++;
   }
+  return this;
 }
 
-void Multiplication::mergeNegativePower() {
+Expression * Multiplication::mergeNegativePower() {
   Multiplication * m = new Multiplication();
   int i = 0;
   while (i < numberOfOperands()) {
@@ -262,7 +267,7 @@ void Multiplication::mergeNegativePower() {
     }
   }
   if (m->numberOfOperands() == 0) {
-    return;
+    return this;
   }
   const Expression * powOperands[2] = {m, new Rational(Integer(-1))};
   Power * p = new Power(powOperands, false);
@@ -271,6 +276,7 @@ void Multiplication::mergeNegativePower() {
   const Expression * multOperand[1] = {p};
   addOperands(multOperand, 1);
   sortChildren();
+  return squashUnaryHierarchy();
 }
 
 template Poincare::Evaluation<float>* Poincare::Multiplication::computeOnComplexAndMatrix<float>(Poincare::Complex<float> const*, Poincare::Evaluation<float>*);

poincare/test/simplify_easy.cpp

@@ -124,6 +124,8 @@ QUIZ_CASE(poincare_simplify_easy) {
   assert_parsed_expression_simplify_to("(x+1)*(x-1)", "-1+x^2");
   assert_parsed_expression_simplify_to("11P/(22P+11P)", "1/3");
   assert_parsed_expression_simplify_to("11/(22P+11P)", "1/(3P)");
+  assert_parsed_expression_simplify_to("A^2*BA^(-2)*B^(-2)", "1/B");
+  assert_parsed_expression_simplify_to("A^(-1)*B^(-1)", "1/(AB)");
   /* This does not work but should not as it is above k_primorial32 = 1*3*5*7*11*... (product of first 32 primes. */
   //assert_parsed_expression_simplify_to("1881676377434183981909562699940347954480361860897069^(1/3)", "123456789123456789");