Units: stop newing Unit objects. pass around pointers to "undeletable" Unit objects in the UnitTable. I think we should move to using indexed units, and pass around the index of the unit in the unittable, or smth like that... ?

(bzr r12679)

4 files changed, 57 insertions, 53 deletions

diff --git a/src/util/expression-evaluator.cpp b/src/util/expression-evaluator.cpp
index 3b7e77c6c..48064e647 100644
--- a/src/util/expression-evaluator.cpp
+++ b/src/util/expression-evaluator.cpp
@@ -48,7 +48,7 @@ EvaluatorToken::EvaluatorToken()
     value.fl = 0;
 }
 
-ExpressionEvaluator::ExpressionEvaluator(const char *string, Unit *unit) :
+ExpressionEvaluator::ExpressionEvaluator(const char *string, Unit const *unit) :
     string(string),
     unit(unit)
 {
@@ -360,7 +360,7 @@ int ExpressionEvaluator::getIdentifierSize(const char *string, int start_offset)
 
 bool ExpressionEvaluator::resolveUnit (const char* identifier,
                                        EvaluatorQuantity *result,
-                                       Unit* unit)
+                                       Unit const* unit)
 {
     if (!unit) {
         result->value = 1;
@@ -371,9 +371,9 @@ bool ExpressionEvaluator::resolveUnit (const char* identifier,
         result->dimension = unit->isAbsolute() ? 1 : 0;
         return true;
     } else if (unit_table.hasUnit(identifier)) {
-        Unit identifier_unit = unit_table.getUnit(identifier);
-        result->value = Quantity::convert(1, *unit, identifier_unit);
-        result->dimension = identifier_unit.isAbsolute() ? 1 : 0;
+        Unit const *identifier_unit = unit_table.getUnit(identifier);
+        result->value = Quantity::convert(1, unit, identifier_unit);
+        result->dimension = identifier_unit->isAbsolute() ? 1 : 0;
         return true;
     } else {
         return false;
diff --git a/src/util/expression-evaluator.h b/src/util/expression-evaluator.h
index 6412dfea7..50ff3fb70 100644
--- a/src/util/expression-evaluator.h
+++ b/src/util/expression-evaluator.h
@@ -133,13 +133,13 @@ public:
 class ExpressionEvaluator
 {
 public:
-    ExpressionEvaluator(const char *string, Unit *unit = NULL);
+    ExpressionEvaluator(const char *string, Unit const *unit = NULL);
     
     EvaluatorQuantity evaluate();
 
 private:
     const char *string;
-    Unit *unit;
+    Unit const *unit;
     
     EvaluatorToken current_token;
     const char *start_of_current_token;
@@ -160,7 +160,7 @@ private:
     static bool isUnitIdentifierStart(gunichar c);
     static int getIdentifierSize(const char *s, int start);
     
-    static bool resolveUnit(const char *identifier, EvaluatorQuantity *result, Unit *unit);
+    static bool resolveUnit(const char *identifier, EvaluatorQuantity *result, Unit const *unit);
     
     void throwError(const char *msg);
 };
diff --git a/src/util/units.cpp b/src/util/units.cpp
index cabbc044d..1023cfb6e 100644
--- a/src/util/units.cpp
+++ b/src/util/units.cpp
@@ -189,15 +189,15 @@ int Unit::defaultDigits() const
     return factor_digits;
 }
 
-bool Unit::compatibleWith(Unit const &u) const
+bool Unit::compatibleWith(Unit const *u) const
 {
     // Percentages
-    if (type == UNIT_TYPE_DIMENSIONLESS || u.type == UNIT_TYPE_DIMENSIONLESS) {
+    if (type == UNIT_TYPE_DIMENSIONLESS || u->type == UNIT_TYPE_DIMENSIONLESS) {
         return true;
     }
 
     // Other units with same type
-    if (type == u.type) {
+    if (type == u->type) {
         return true;
     }
 
@@ -254,30 +254,30 @@ void UnitTable::addUnit(Unit const &u, bool primary)
     }
 }
 
-Unit const &UnitTable::getUnit(char const *abbr) const
+Unit const *UnitTable::getUnit(char const *abbr) const
 {
     UnitCodeMap::const_iterator f = _unit_map.find(make_unit_code(abbr));
     if (f != _unit_map.end()) {
-        return *f->second;
+        return &(*f->second);
     }
-    return _empty_unit;
+    return &_empty_unit;
 }
 
-Unit const &UnitTable::getUnit(Glib::ustring const &unit_abbr) const
+Unit const *UnitTable::getUnit(Glib::ustring const &unit_abbr) const
 {
     return getUnit(unit_abbr.c_str());
 }
-Unit const &UnitTable::getUnit(SVGLength::Unit u) const
+Unit const *UnitTable::getUnit(SVGLength::Unit u) const
 {
     if (u == 0 || u > SVGLength::LAST_UNIT) {
-        return _empty_unit;
+        return &_empty_unit;
     }
 
     UnitCodeMap::const_iterator f = _unit_map.find(svg_length_lookup[u]);
     if (f != _unit_map.end()) {
-        return *f->second;
+        return &(*f->second);
     }
-    return _empty_unit;
+    return &_empty_unit;
 }
 
 Quantity UnitTable::parseQuantity(Glib::ustring const &q) const
@@ -303,6 +303,7 @@ Quantity UnitTable::parseQuantity(Glib::ustring const &q) const
     return qty;
 }
 
+/* UNSAFE while passing around pointers to the Unit objects in this table 
 bool UnitTable::deleteUnit(Unit const &u)
 {
     bool deleted = false;
@@ -318,6 +319,7 @@ bool UnitTable::deleteUnit(Unit const &u)
     }
     return deleted;
 }
+*/
 
 bool UnitTable::hasUnit(Glib::ustring const &unit) const
 {
@@ -418,23 +420,23 @@ void UnitParser::on_end_element(Ctx &ctx, Glib::ustring const &name)
     }
 }
 
-Quantity::Quantity(double q, Unit const &u)
+Quantity::Quantity(double q, Unit const *u)
+  : unit(u)
+  , quantity(q)
 {
-    unit = new Unit(u);
-    quantity = q;
 }
 Quantity::Quantity(double q, Glib::ustring const &u)
+  : unit(unit_table.getUnit(u.c_str()))
+  , quantity(q)
 {
-    unit = new Unit(unit_table.getUnit(u.c_str()));
-    quantity = q;
 }
 Quantity::Quantity(double q, char const *u)
+  : unit(unit_table.getUnit(u))
+  , quantity(q)
 {
-    unit = new Unit(unit_table.getUnit(u));
-    quantity = q;
 }
 
-bool Quantity::compatibleWith(Unit const &u) const
+bool Quantity::compatibleWith(Unit const *u) const
 {
     return unit->compatibleWith(u);
 }
@@ -447,9 +449,9 @@ bool Quantity::compatibleWith(char const *u) const
     return compatibleWith(unit_table.getUnit(u));
 }
 
-double Quantity::value(Unit const &u) const
+double Quantity::value(Unit const *u) const
 {
-    return convert(quantity, *unit, u);
+    return convert(quantity, unit, u);
 }
 double Quantity::value(Glib::ustring const &u) const
 {
@@ -460,36 +462,36 @@ double Quantity::value(char const *u) const
     return value(unit_table.getUnit(u));
 }
 
-Glib::ustring Quantity::string(Unit const &u) const {
+Glib::ustring Quantity::string(Unit const *u) const {
     return Glib::ustring::format(std::fixed, std::setprecision(2), value(u)) + " " + unit->abbr;
 }
 Glib::ustring Quantity::string(Glib::ustring const &u) const {
     return string(unit_table.getUnit(u.c_str()));
 }
 Glib::ustring Quantity::string() const {
-    return string(*unit);
+    return string(unit);
 }
 
-double Quantity::convert(double from_dist, Unit const &from, Unit const &to)
+double Quantity::convert(double from_dist, Unit const *from, Unit const *to)
 {
     // Percentage
-    if (to.type == UNIT_TYPE_DIMENSIONLESS) {
-        return from_dist * to.factor;
+    if (to->type == UNIT_TYPE_DIMENSIONLESS) {
+        return from_dist * to->factor;
     }
     
     // Incompatible units
-    if (from.type != to.type) {
+    if (from->type != to->type) {
         return -1;
     }
     
     // Compatible units
-    return from_dist * from.factor / to.factor;
+    return from_dist * from->factor / to->factor;
 }
-double Quantity::convert(double from_dist, Glib::ustring const &from, Unit const &to)
+double Quantity::convert(double from_dist, Glib::ustring const &from, Unit const *to)
 {
     return convert(from_dist, unit_table.getUnit(from.c_str()), to);
 }
-double Quantity::convert(double from_dist, Unit const &from, Glib::ustring const &to)
+double Quantity::convert(double from_dist, Unit const *from, Glib::ustring const &to)
 {
     return convert(from_dist, from, unit_table.getUnit(to.c_str()));
 }
@@ -508,7 +510,7 @@ bool Quantity::operator<(Quantity const &other) const
         g_warning("Incompatible units");
         return false;
     }
-    return quantity < other.value(*unit);
+    return quantity < other.value(unit);
 }
 bool Quantity::operator==(Quantity const &other) const
 {
diff --git a/src/util/units.h b/src/util/units.h
index 7566715d8..e1addaa24 100644
--- a/src/util/units.h
+++ b/src/util/units.h
@@ -58,7 +58,7 @@ public:
     int            defaultDigits() const;
 
     /** Checks if a unit is compatible with the specified unit. */
-    bool           compatibleWith(Unit const &u) const;
+    bool           compatibleWith(Unit const *u) const;
     bool           compatibleWith(Glib::ustring const &) const;
     bool           compatibleWith(char const *) const;
 
@@ -84,29 +84,30 @@ public:
     double quantity;
     
     /** Initialize a quantity. */
-    Quantity(double q, Unit const &u);
+    Quantity(double q, Unit const *u);
     Quantity(double q, Glib::ustring const &u);
     Quantity(double q, char const *u);
     
     /** Checks if a quantity is compatible with the specified unit. */
-    bool compatibleWith(Unit const &u) const;
+    bool compatibleWith(Unit const *u) const;
     bool compatibleWith(Glib::ustring const &u) const;
     bool compatibleWith(char const *u) const;
     
     /** Return the quantity's value in the specified unit. */
-    double value(Unit const &u) const;
+    double value(Unit const *u) const;
     double value(Glib::ustring const &u) const;
     double value(char const *u) const;
     
     /** Return a printable string of the value in the specified unit. */
-    Glib::ustring string(Unit const &u) const;
+    Glib::ustring string(Unit const *u) const;
     Glib::ustring string(Glib::ustring const &u) const;
     Glib::ustring string() const;
     
-    /** Convert distances. */
-    static double convert(double from_dist, Unit const &from, Unit const &to);
-    static double convert(double from_dist, Glib::ustring const &from, Unit const &to);
-    static double convert(double from_dist, Unit const &from, Glib::ustring const &to);
+    /** Convert distances. 
+       no NULL check is performed on the passed pointers to Unit objects!  */
+    static double convert(double from_dist, Unit const *from, Unit const *to);
+    static double convert(double from_dist, Glib::ustring const &from, Unit const *to);
+    static double convert(double from_dist, Unit const *from, Glib::ustring const &to);
     static double convert(double from_dist, Glib::ustring const &from, Glib::ustring const &to);
     static double convert(double from_dist, char const *from, char const *to);
 
@@ -132,17 +133,18 @@ public:
     void    addUnit(Unit const &u, bool primary);
 
     /** Retrieve a given unit based on its string identifier */
-    Unit const &getUnit(Glib::ustring const &name) const;
-    Unit const &getUnit(char const *name) const;
+    Unit const *getUnit(Glib::ustring const &name) const;
+    Unit const *getUnit(char const *name) const;
     
     /** Retrieve a given unit based on its SVGLength unit */
-    Unit const &getUnit(SVGLength::Unit u) const;
+    Unit const *getUnit(SVGLength::Unit u) const;
     
     /** Retrieve a quantity based on its string identifier */
     Quantity parseQuantity(Glib::ustring const &q) const;
 
-    /** Remove a unit definition from the given unit type table */
-    bool    deleteUnit(Unit const &u);
+    /** Remove a unit definition from the given unit type table * /
+     * DISABLED, unsafe with the current passing around pointers to Unit objects in this table */
+    //bool    deleteUnit(Unit const &u);
 
     /** Returns true if the given string 'name' is a valid unit in the table */
     bool    hasUnit(Glib::ustring const &name) const;