From f115ed4bb1137df67ad2f248acb2774605c9c75c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Br=C3=A6strup=20Sayoc?= Date: Thu, 30 Jan 2025 22:45:48 +0100 Subject: [PATCH] Remove magic numbers and hard type enums in DimensionFormatter.h.h - Remove currently present magic numbers - Hard type enums, so magic numbers can no longer be introduced. We don't want people to introduce magic numbers. --- src/Mod/TechDraw/App/AppTechDrawPy.cpp | 4 +-- src/Mod/TechDraw/App/DimensionFormatter.cpp | 34 +++++++++---------- src/Mod/TechDraw/App/DimensionFormatter.h | 21 ++++++++---- src/Mod/TechDraw/App/DrawViewDimension.cpp | 8 ++--- src/Mod/TechDraw/App/DrawViewDimension.h | 13 ++++--- src/Mod/TechDraw/Gui/CommandExtensionPack.cpp | 3 +- src/Mod/TechDraw/Gui/QGIViewDimension.cpp | 14 ++++---- 7 files changed, 56 insertions(+), 41 deletions(-) diff --git a/src/Mod/TechDraw/App/AppTechDrawPy.cpp b/src/Mod/TechDraw/App/AppTechDrawPy.cpp index fe6fe56439..a41bb697c5 100644 --- a/src/Mod/TechDraw/App/AppTechDrawPy.cpp +++ b/src/Mod/TechDraw/App/AppTechDrawPy.cpp @@ -732,9 +732,9 @@ private: std::string sDimText; //this is the same code as in QGIViewDimension::updateDim if (dvd->isMultiValueSchema()) { - sDimText = dvd->getFormattedDimensionValue(0); //don't format multis + sDimText = dvd->getFormattedDimensionValue(DimensionFormatter::Format::UNALTERED); //don't format multis } else { - sDimText = dvd->getFormattedDimensionValue(1); + sDimText = dvd->getFormattedDimensionValue(DimensionFormatter::Format::FORMATTED); } char* dimText = &sDimText[0u]; //hack for const-ness float gap = 5.0; //hack. don't know font size here. diff --git a/src/Mod/TechDraw/App/DimensionFormatter.cpp b/src/Mod/TechDraw/App/DimensionFormatter.cpp index c6190473cc..c3bc820463 100644 --- a/src/Mod/TechDraw/App/DimensionFormatter.cpp +++ b/src/Mod/TechDraw/App/DimensionFormatter.cpp @@ -29,9 +29,11 @@ #include #include +#include "DrawViewDimension.h" #include "DimensionFormatter.h" #include "Preferences.h" +// TODO: Cyclic dependency issue with DrawViewDimension using namespace TechDraw; @@ -50,14 +52,9 @@ bool DimensionFormatter::isMultiValueSchema() const return false; } -// Todo: make this enum -//partial = 0 return the unaltered user string from the Units subsystem -//partial = 1 return value formatted according to the format spec and preferences for -// useAltDecimals and showUnits -//partial = 2 return only the unit of measure std::string DimensionFormatter::formatValue(const qreal value, const QString& qFormatSpec, - const int partial, + const Format partial, const bool isDim) const { // Base::Console().Message("DF::formatValue() - %s isRestoring: %d\n", @@ -92,7 +89,7 @@ std::string DimensionFormatter::formatValue(const qreal value, QString qBasicUnit = QString::fromStdString(Base::UnitsApi::getBasicLengthUnit()); QString formattedValue; - if (isMultiValueSchema() && partial == 0) { + if (isMultiValueSchema() && partial == Format::UNALTERED) { //handle multi value schemes (yd/ft/in, dms, etc). don't even try to use Alt Decimals or hide units qMultiValueStr = formatPrefix + qUserString + formatSuffix; return qMultiValueStr.toStdString(); @@ -156,12 +153,12 @@ std::string DimensionFormatter::formatValue(const qreal value, //formattedValue is now in formatSpec format with local decimal separator std::string formattedValueString = formattedValue.toStdString(); - if (partial == 0) { //prefix + unit subsystem string + suffix + if (partial == Format::UNALTERED) { // prefix + unit subsystem string + suffix return formatPrefix.toStdString() + qUserString.toStdString() + formatSuffix.toStdString(); } - else if (partial == 1) { // prefix number[unit] suffix + else if (partial == Format::FORMATTED) { if (angularMeasure) { //always insert unit after value return formatPrefix.toStdString() + formattedValueString + "°" + @@ -189,7 +186,7 @@ std::string DimensionFormatter::formatValue(const qreal value, formatSuffix.toStdString(); } } - else if (partial == 2) { // just the unit + else if (partial == Format::UNIT) { if (angularMeasure) { return qBasicUnit.toStdString(); } @@ -207,7 +204,7 @@ std::string DimensionFormatter::formatValue(const qreal value, //! get the formatted OverTolerance value // wf: is this a leftover from when we only had 1 tolerance instead of over/under? -std::string DimensionFormatter::getFormattedToleranceValue(const int partial) const +std::string DimensionFormatter::getFormattedToleranceValue(const Format partial) const { QString FormatSpec = QString::fromUtf8(m_dimension->FormatSpecOverTolerance.getStrValue().data()); QString ToleranceString; @@ -224,7 +221,7 @@ std::string DimensionFormatter::getFormattedToleranceValue(const int partial) co } //! get formatted over and under tolerances -std::pair DimensionFormatter::getFormattedToleranceValues(const int partial) const +std::pair DimensionFormatter::getFormattedToleranceValues(const Format partial) const { QString underFormatSpec = QString::fromUtf8(m_dimension->FormatSpecUnderTolerance.getStrValue().data()); QString overFormatSpec = QString::fromUtf8(m_dimension->FormatSpecOverTolerance.getStrValue().data()); @@ -252,7 +249,7 @@ std::pair DimensionFormatter::getFormattedToleranceVal } //partial = 2 unit only -std::string DimensionFormatter::getFormattedDimensionValue(const int partial) const +std::string DimensionFormatter::getFormattedDimensionValue(const Format partial) const { QString qFormatSpec = QString::fromUtf8(m_dimension->FormatSpec.getStrValue().data()); @@ -271,19 +268,21 @@ std::string DimensionFormatter::getFormattedDimensionValue(const int partial) co // (OverTolerance != 0.0 (so a tolerance has been specified) or // ArbitraryTolerances are specified) // concatenate the tolerance to dimension + + // TODO: why all this QString if returned as std::string??? if (m_dimension->EqualTolerance.getValue() && !m_dimension->TheoreticalExact.getValue() && (!DrawUtil::fpCompare(m_dimension->OverTolerance.getValue(), 0.0) || m_dimension->ArbitraryTolerances.getValue())) { QString labelText = QString::fromUtf8(formatValue(m_dimension->getDimValue(), qFormatSpec, - 1, + Format::FORMATTED, true).c_str()); //just the number pref/spec[unit]/suf QString unitText = QString::fromUtf8(formatValue(m_dimension->getDimValue(), qFormatSpec, - 2, + Format::UNIT, false).c_str()); //just the unit - QString tolerance = QString::fromStdString(getFormattedToleranceValue(1).c_str()); + QString tolerance = QString::fromStdString(getFormattedToleranceValue(Format::FORMATTED).c_str()); // tolerance might start with a plus sign that we don't want, so cut it off // note plus sign is not at pos = 0! @@ -294,7 +293,8 @@ std::string DimensionFormatter::getFormattedDimensionValue(const int partial) co QStringLiteral(" \xC2\xB1 ") + // +/- symbol tolerance).toStdString(); - if (partial == 2) { + // Unreachable code?? + if (partial == Format::UNIT) { return unitText.toStdString(); } diff --git a/src/Mod/TechDraw/App/DimensionFormatter.h b/src/Mod/TechDraw/App/DimensionFormatter.h index e12430fa28..0afeb7ad97 100644 --- a/src/Mod/TechDraw/App/DimensionFormatter.h +++ b/src/Mod/TechDraw/App/DimensionFormatter.h @@ -25,26 +25,35 @@ #include -#include +//#include Cyclic dependency issue! namespace TechDraw { +class DrawViewDimension; +//TODO: Why is this a class if it has no state??? class TechDrawExport DimensionFormatter { public: + enum class Format : int { + UNALTERED, // return the unaltered user string from the Units subsystem + FORMATTED, // return value formatted according to the format spec and + // preferences for useAltDecimals and showUnits + UNIT // return only the unit of measure + }; + DimensionFormatter() {} DimensionFormatter(DrawViewDimension* dim) { m_dimension = dim; } ~DimensionFormatter() = default; - void setDimension(DrawViewDimension* dim) { m_dimension = dim; } + //void setDimension(DrawViewDimension* dim) { m_dimension = dim; } bool isMultiValueSchema() const; std::string formatValue(const qreal value, const QString& qFormatSpec, - const int partial, + const Format partial, const bool isDim) const; - std::string getFormattedToleranceValue(const int partial) const; - std::pair getFormattedToleranceValues(const int partial) const; - std::string getFormattedDimensionValue(const int partial) const; + std::string getFormattedToleranceValue(const Format partial) const; + std::pair getFormattedToleranceValues(const Format partial) const; + std::string getFormattedDimensionValue(const Format partial) const; QStringList getPrefixSuffixSpec(const QString& fSpec) const; std::string getDefaultFormatSpec(bool isToleranceFormat) const; bool isTooSmall(const double value, const QString& formatSpec) const; diff --git a/src/Mod/TechDraw/App/DrawViewDimension.cpp b/src/Mod/TechDraw/App/DrawViewDimension.cpp index ca78906b04..a2e5244955 100644 --- a/src/Mod/TechDraw/App/DrawViewDimension.cpp +++ b/src/Mod/TechDraw/App/DrawViewDimension.cpp @@ -605,7 +605,7 @@ bool DrawViewDimension::isMultiValueSchema() const } std::string -DrawViewDimension::formatValue(qreal value, QString qFormatSpec, int partial, bool isDim) +DrawViewDimension::formatValue(qreal value, QString qFormatSpec, DimensionFormatter::Format partial, bool isDim) { return m_formatter->formatValue(value, qFormatSpec, partial, isDim); } @@ -622,19 +622,19 @@ bool DrawViewDimension::haveTolerance() return false; } -std::string DrawViewDimension::getFormattedToleranceValue(int partial) +std::string DrawViewDimension::getFormattedToleranceValue(DimensionFormatter::Format partial) { return m_formatter->getFormattedToleranceValue(partial); } ////get over and under tolerances -std::pair DrawViewDimension::getFormattedToleranceValues(int partial) +std::pair DrawViewDimension::getFormattedToleranceValues(DimensionFormatter::Format partial) { return m_formatter->getFormattedToleranceValues(partial); } ////partial = 2 unit only -std::string DrawViewDimension::getFormattedDimensionValue(int partial) +std::string DrawViewDimension::getFormattedDimensionValue(DimensionFormatter::Format partial) { return m_formatter->getFormattedDimensionValue(partial); } diff --git a/src/Mod/TechDraw/App/DrawViewDimension.h b/src/Mod/TechDraw/App/DrawViewDimension.h index 5ee5380e34..1493a9039d 100644 --- a/src/Mod/TechDraw/App/DrawViewDimension.h +++ b/src/Mod/TechDraw/App/DrawViewDimension.h @@ -29,6 +29,7 @@ #include +#include "DimensionFormatter.h" #include "DimensionGeometry.h" #include "DimensionReferences.h" #include "DrawUtil.h" @@ -44,9 +45,11 @@ class Measurement; namespace TechDraw { class DrawViewPart; -class DimensionFormatter; class GeometryMatcher; class DimensionAutoCorrect; +using DF = DimensionFormatter; + +//TODO: Cyclic dependency issue with DimensionFormatter class TechDrawExport DrawViewDimension: public TechDraw::DrawView { @@ -123,11 +126,11 @@ public: // return PyObject as DrawViewDimensionPy PyObject* getPyObject() override; - virtual std::string getFormattedToleranceValue(int partial); - virtual std::pair getFormattedToleranceValues(int partial = 0); - virtual std::string getFormattedDimensionValue(int partial = 0); + virtual std::string getFormattedToleranceValue(DF::Format partial); + virtual std::pair getFormattedToleranceValues(DF::Format partial = DF::Format::UNALTERED); + virtual std::string getFormattedDimensionValue(DF::Format partial = DF::Format::UNALTERED); virtual std::string - formatValue(qreal value, QString qFormatSpec, int partial = 0, bool isDim = true); + formatValue(qreal value, QString qFormatSpec, DF::Format partial = DF::Format::UNALTERED, bool isDim = true); virtual bool haveTolerance(); diff --git a/src/Mod/TechDraw/Gui/CommandExtensionPack.cpp b/src/Mod/TechDraw/Gui/CommandExtensionPack.cpp index ad158a3324..858c6d3492 100644 --- a/src/Mod/TechDraw/Gui/CommandExtensionPack.cpp +++ b/src/Mod/TechDraw/Gui/CommandExtensionPack.cpp @@ -1981,9 +1981,10 @@ void CmdTechDrawExtensionArcLengthAnnotation::activated(int iMsg) // Use virtual dimension view helper to format resulting value TechDraw::DrawViewDimension helperDim; + using Format = DimensionFormatter::Format; std::string valueStr = helperDim.formatValue(totalLength, QString::fromUtf8(helperDim.FormatSpec.getStrValue().data()), - helperDim.isMultiValueSchema() ? 0 : 1); + helperDim.isMultiValueSchema() ? Format::UNALTERED : Format::FORMATTED); balloon->Text.setValue("◠ " + valueStr); // Set balloon format to be referencing dimension-like diff --git a/src/Mod/TechDraw/Gui/QGIViewDimension.cpp b/src/Mod/TechDraw/Gui/QGIViewDimension.cpp index a4d63e3687..b8b2fdefe7 100644 --- a/src/Mod/TechDraw/Gui/QGIViewDimension.cpp +++ b/src/Mod/TechDraw/Gui/QGIViewDimension.cpp @@ -69,6 +69,7 @@ using namespace TechDraw; using namespace TechDrawGui; +using Format = DimensionFormatter::Format; enum SnapMode { @@ -522,19 +523,19 @@ void QGIDatumLabel::setToleranceString() std::pair labelTexts, unitTexts; if (dim->ArbitraryTolerances.getValue()) { - labelTexts = dim->getFormattedToleranceValues(1);//copy tolerance spec + labelTexts = dim->getFormattedToleranceValues(Format::FORMATTED);//copy tolerance spec unitTexts.first = ""; unitTexts.second = ""; } else { if (dim->isMultiValueSchema()) { - labelTexts = dim->getFormattedToleranceValues(0);//don't format multis + labelTexts = dim->getFormattedToleranceValues(Format::UNALTERED);//don't format multis unitTexts.first = ""; unitTexts.second = ""; } else { - labelTexts = dim->getFormattedToleranceValues(1);// prefix value [unit] postfix - unitTexts = dim->getFormattedToleranceValues(2); //just the unit + labelTexts = dim->getFormattedToleranceValues(Format::FORMATTED);// prefix value [unit] postfix + unitTexts = dim->getFormattedToleranceValues(Format::UNIT); //just the unit } } @@ -831,10 +832,11 @@ void QGIViewDimension::updateDim() } QString labelText = - QString::fromUtf8(dim->getFormattedDimensionValue(1).c_str());// pre value [unit] post + // what about fromStdString? + QString::fromUtf8(dim->getFormattedDimensionValue(Format::FORMATTED).c_str());// pre value [unit] post if (dim->isMultiValueSchema()) { labelText = - QString::fromUtf8(dim->getFormattedDimensionValue(0).c_str());//don't format multis + QString::fromUtf8(dim->getFormattedDimensionValue(Format::UNALTERED).c_str());//don't format multis } QFont font = datumLabel->getFont();