From 913c30429cabe1bc6d7788a96fa4975ca2d4ff91 Mon Sep 17 00:00:00 2001 From: Ladislav Michl Date: Thu, 10 Apr 2025 17:43:41 +0200 Subject: [PATCH] Base: Quantity: use isDimensionless whenever feasible Quantity is often queried for Unit just to see if it has a dimension. Ask Quantity directly using isDimensionless() method and modify that method not to care about Quantity value validity; no user was ever asking for value validity. --- src/App/Expression.cpp | 2 +- src/App/PropertyUnits.cpp | 12 ++++---- src/Base/Quantity.cpp | 10 ++----- src/Base/Quantity.h | 4 +-- src/Base/QuantityPyImp.cpp | 38 +++++++++--------------- src/Gui/Dialogs/DlgExpressionInput.cpp | 4 +-- src/Gui/InputField.cpp | 6 ++-- src/Gui/QuantitySpinBox.cpp | 2 +- src/Mod/Sketcher/Gui/EditDatumDialog.cpp | 4 +-- tests/src/Base/Quantity.cpp | 7 ----- 10 files changed, 31 insertions(+), 58 deletions(-) diff --git a/src/App/Expression.cpp b/src/App/Expression.cpp index 6e9db2bc83..9275e678b1 100644 --- a/src/App/Expression.cpp +++ b/src/App/Expression.cpp @@ -488,7 +488,7 @@ static inline Quantity pyToQuantity(const Py::Object &pyobj, } Py::Object pyFromQuantity(const Quantity &quantity) { - if(!quantity.getUnit().isEmpty()) + if (!quantity.isDimensionless()) return Py::asObject(new QuantityPy(new Quantity(quantity))); double v = quantity.getValue(); long l; diff --git a/src/App/PropertyUnits.cpp b/src/App/PropertyUnits.cpp index 300c3ca786..ca38aed84a 100644 --- a/src/App/PropertyUnits.cpp +++ b/src/App/PropertyUnits.cpp @@ -105,13 +105,12 @@ void PropertyQuantity::setPyObject(PyObject* value) else { Base::Quantity quant = createQuantityFromPy(value); - Unit unit = quant.getUnit(); - if (unit.isEmpty()) { + if (quant.isDimensionless()) { PropertyFloat::setValue(quant.getValue()); return; } - if (unit != _Unit) { + if (_Unit != quant.getUnit()) { throw Base::UnitsMismatchError("Not matching Unit!"); } @@ -123,7 +122,7 @@ void PropertyQuantity::setPathValue(const ObjectIdentifier& /*path*/, const boos { auto q = App::anyToQuantity(value); aboutToSetValue(); - if (!q.getUnit().isEmpty()) { + if (!q.isDimensionless()) { _Unit = q.getUnit(); } _dValue = q.getValue(); @@ -187,7 +186,6 @@ void PropertyQuantityConstraint::setPyObject(PyObject* value) { Base::Quantity quant = createQuantityFromPy(value); - Unit unit = quant.getUnit(); double temp = quant.getValue(); if (_ConstStruct) { if (temp > _ConstStruct->UpperBound) { @@ -199,12 +197,12 @@ void PropertyQuantityConstraint::setPyObject(PyObject* value) } quant.setValue(temp); - if (unit.isEmpty()) { + if (quant.isDimensionless()) { PropertyFloat::setValue(quant.getValue()); // clazy:exclude=skipped-base-method return; } - if (unit != _Unit) { + if (_Unit != quant.getUnit()) { throw Base::UnitsMismatchError("Not matching Unit!"); } diff --git a/src/Base/Quantity.cpp b/src/Base/Quantity.cpp index efa8c37a3c..babbe4a4c2 100644 --- a/src/Base/Quantity.cpp +++ b/src/Base/Quantity.cpp @@ -183,7 +183,7 @@ Quantity Quantity::operator/(double factor) const Quantity Quantity::pow(const Quantity& other) const { - if (!other.myUnit.isEmpty()) { + if (!other.isDimensionless()) { throw Base::UnitsMismatchError("Quantity::pow(): exponent must not have a unit"); } @@ -273,7 +273,7 @@ std::string Quantity::getSafeUserString() const /// true if it has a number without a unit bool Quantity::isDimensionless() const { - return isValid() && myUnit.isEmpty(); + return myUnit.isEmpty(); } /// true if it has a specific unit or no dimension. @@ -282,12 +282,6 @@ bool Quantity::isDimensionlessOrUnit(const Unit& unit) const return isDimensionless() || myUnit == unit; } -// true if it has a number and a valid unit -bool Quantity::isQuantity() const -{ - return isValid() && !myUnit.isEmpty(); -} - // true if it has a number with or without a unit bool Quantity::isValid() const { diff --git a/src/Base/Quantity.h b/src/Base/Quantity.h index ed0f947ac9..657f3af1b4 100644 --- a/src/Base/Quantity.h +++ b/src/Base/Quantity.h @@ -191,12 +191,10 @@ public: double getValueAs(const Quantity&) const; - /// true if it has a number without a unit + /// true if it has no unit bool isDimensionless() const; /// true if it has a specific unit or no dimension. bool isDimensionlessOrUnit(const Unit& unit) const; - /// true if it has a number and a valid unit - bool isQuantity() const; /// true if it has a number with or without a unit bool isValid() const; /// sets the quantity invalid diff --git a/src/Base/QuantityPyImp.cpp b/src/Base/QuantityPyImp.cpp index d832945b5e..1fea8807c6 100644 --- a/src/Base/QuantityPyImp.cpp +++ b/src/Base/QuantityPyImp.cpp @@ -43,19 +43,15 @@ using Base::Quantity; // returns a string which represents the object e.g. when printed in python std::string QuantityPy::representation() const { - std::stringstream ret; - - double val = getQuantityPtr()->getValue(); - Unit unit = getQuantityPtr()->getUnit(); - + std::stringstream ss; // Use Python's implementation to repr() a float - Py::Float flt(val); - ret << static_cast(flt.repr()); - if (!unit.isEmpty()) { - ret << " " << unit.getString(); + Py::Float flt(getQuantityPtr()->getValue()); + ss << static_cast(flt.repr()); + if (!getQuantityPtr()->isDimensionless()) { + ss << " " << getQuantityPtr()->getUnit().getString(); } - return ret.str(); + return ss.str(); } PyObject* QuantityPy::toStr(PyObject* args) const @@ -66,17 +62,16 @@ PyObject* QuantityPy::toStr(PyObject* args) const } double val = getQuantityPtr()->getValue(); - Unit unit = getQuantityPtr()->getUnit(); - std::stringstream ret; - ret.precision(prec); - ret.setf(std::ios::fixed, std::ios::floatfield); - ret << val; - if (!unit.isEmpty()) { - ret << " " << unit.getString(); + std::stringstream ss; + ss.precision(prec); + ss.setf(std::ios::fixed, std::ios::floatfield); + ss << val; + if (!getQuantityPtr()->isDimensionless()) { + ss << " " << getQuantityPtr()->getUnit().getString(); } - return Py_BuildValue("s", ret.str().c_str()); + return Py_BuildValue("s", ss.str().c_str()); } PyObject* QuantityPy::PyMake(PyTypeObject* /*unused*/, PyObject* /*unused*/, PyObject* /*unused*/) @@ -279,11 +274,6 @@ PyObject* QuantityPy::getValueAs(PyObject* args) const } const auto qpUnit = qPtr->getUnit(); - if (qpUnit.isEmpty()) { - err("QuantityPtr returned empty unit"); - return false; - } - if (const auto qUnit = quant.getUnit(); qUnit != qpUnit) { err("Unit mismatch (`" + qUnit.getString() + "` != `" + qpUnit.getString() + "`)"); return false; @@ -301,7 +291,7 @@ PyObject* QuantityPy::getValueAs(PyObject* args) const } const auto quant = optQuant.value(); - if (quant.isQuantity()) { + if (!quant.isDimensionless()) { if (!checkQuant(quant)) { return nullptr; } diff --git a/src/Gui/Dialogs/DlgExpressionInput.cpp b/src/Gui/Dialogs/DlgExpressionInput.cpp index 336bbbd2e5..b8ee7c36b1 100644 --- a/src/Gui/Dialogs/DlgExpressionInput.cpp +++ b/src/Gui/Dialogs/DlgExpressionInput.cpp @@ -295,13 +295,13 @@ void DlgExpressionInput::checkExpression(const QString& text) auto msg = value.getUserString(); if (!impliedUnit.isEmpty()) { - if (!value.getUnit().isEmpty() && value.getUnit() != impliedUnit) + if (!value.isDimensionless() && value.getUnit() != impliedUnit) throw Base::UnitsMismatchError("Unit mismatch between result and required unit"); value.setUnit(impliedUnit); } - else if (!value.getUnit().isEmpty()) { + else if (!value.isDimensionless()) { msg += " (Warning: unit discarded)"; QPalette p(ui->msg->palette()); diff --git a/src/Gui/InputField.cpp b/src/Gui/InputField.cpp index e203e0bc69..d6ca20d1a3 100644 --- a/src/Gui/InputField.cpp +++ b/src/Gui/InputField.cpp @@ -278,11 +278,11 @@ void InputField::newInput(const QString & text) return; } - if (res.getUnit().isEmpty()) + if (res.isDimensionless()) res.setUnit(this->actUnit); // check if unit fits! - if(!actUnit.isEmpty() && !res.getUnit().isEmpty() && actUnit != res.getUnit()){ + if (!actUnit.isEmpty() && !res.isDimensionless() && actUnit != res.getUnit()){ if (iconLabel->isHidden()) { iconLabel->setVisible(true); } @@ -644,7 +644,7 @@ void InputField::focusInEvent(QFocusEvent *event) void InputField::focusOutEvent(QFocusEvent *event) { try { - if (Quantity::parse(this->text().toStdString()).getUnit().isEmpty()) { + if (Quantity::parse(this->text().toStdString()).isDimensionless()) { // if user didn't enter a unit, we virtually compensate // the multiplication factor induced by user unit system double factor; diff --git a/src/Gui/QuantitySpinBox.cpp b/src/Gui/QuantitySpinBox.cpp index 59f88073bc..4631b45c75 100644 --- a/src/Gui/QuantitySpinBox.cpp +++ b/src/Gui/QuantitySpinBox.cpp @@ -224,7 +224,7 @@ public: ok = parseString(copy, res, value, path); // If result does not have unit: add default unit - if (res.getUnit().isEmpty()){ + if (res.isDimensionless()) { res.setUnit(unit); } diff --git a/src/Mod/Sketcher/Gui/EditDatumDialog.cpp b/src/Mod/Sketcher/Gui/EditDatumDialog.cpp index e04bd45b48..fd8337da31 100644 --- a/src/Mod/Sketcher/Gui/EditDatumDialog.cpp +++ b/src/Mod/Sketcher/Gui/EditDatumDialog.cpp @@ -201,8 +201,8 @@ int EditDatumDialog::exec(bool atCursor) void EditDatumDialog::accepted() { Base::Quantity newQuant = ui_ins_datum->labelEdit->value(); - if (newQuant.isQuantity() || (Constr->Type == Sketcher::SnellsLaw && newQuant.isDimensionless()) - || (Constr->Type == Sketcher::Weight && newQuant.isDimensionless())) { + if (Constr->Type == Sketcher::SnellsLaw || Constr->Type == Sketcher::Weight + || !newQuant.isDimensionless()) { // save the value for the history ui_ins_datum->labelEdit->pushToHistory(); diff --git a/tests/src/Base/Quantity.cpp b/tests/src/Base/Quantity.cpp index 7e555d7e71..d333236271 100644 --- a/tests/src/Base/Quantity.cpp +++ b/tests/src/Base/Quantity.cpp @@ -27,13 +27,6 @@ TEST(BaseQuantity, TestParse) EXPECT_THROW(auto rew [[maybe_unused]] = Quantity::parse("1,234,500.12 kg"), ParserError); } -TEST(BaseQuantity, TestDim) -{ - const Quantity q1 {0, Unit::Area}; - - EXPECT_EQ(q1.isQuantity(), true); -} - TEST(BaseQuantity, TestNoDim) { const Quantity q1 {};