Fix crash when changing yeast measurement type to count

meson.build

@@ -812,7 +812,6 @@ commonSourceFiles = files([
    'src/widgets/SelectionControl.cpp',
    'src/widgets/SmartAmountSettings.cpp',
    'src/widgets/SmartAmounts.cpp',
-   'src/widgets/SmartCheckBox.cpp',
    'src/widgets/SmartDigitWidget.cpp',
    'src/widgets/SmartField.cpp',
    'src/widgets/SmartLabel.cpp',
@@ -996,7 +995,6 @@ mocHeaders = files([
    'src/widgets/InfoButton.h',
    'src/widgets/InfoText.h',
    'src/widgets/SelectionControl.h',
-   'src/widgets/SmartCheckBox.h',
    'src/widgets/SmartDigitWidget.h',
    'src/widgets/SmartLabel.h',
    'src/widgets/SmartLineEdit.h',

src/BtFieldType.h

@@ -1,5 +1,5 @@
 /*======================================================================================================================
- * BtFieldType.h is part of Brewken, and is copyright the following authors 2022-2023:
+ * BtFieldType.h is part of Brewken, and is copyright the following authors 2022-2024:
  *   • Matt Young <[email protected]>
  *
  * Brewken is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License
@@ -44,6 +44,9 @@ enum class NonPhysicalQuantity {
     *
     * This is also used for "number of times" something has been done or is to be done.  Eg number of times a yeast
     * sample has been cultured or reused.  Similarly, it's used for the number of fermentation stages in a Recipe.
+    *
+    * NOTE, however, this is NOT used for "number of things" (eg number of packets of yeast).  For that, we use
+    * Measurement::PhysicalQuantity::Count, for the reasons explained in measurement/PhysicalQuantity.h.
     */
    OrdinalNumeral,
    /**

src/BtTreeModel.cpp

@@ -499,12 +499,15 @@ bool BtTreeModel::removeRows(int row, int count, const QModelIndex & parent) {
 QModelIndex BtTreeModel::findElement(NamedEntity * thing, BtTreeItem * parent) {
    BtTreeItem * pItem = parent ? parent : this->rootItem->child(0);
 
-   qDebug() << Q_FUNC_INFO << "Find" << thing << "in" << pItem;
 
    if (! thing) {
       return createIndex(0, 0, pItem);
    }
 
+   if (pItem) {
+      qDebug() << Q_FUNC_INFO << "Find" << *thing << "in" << pItem->name();
+   }
+
    QList<BtTreeItem *> folders;
    folders.append(pItem);
 

src/CMakeLists.txt

@@ -256,7 +256,6 @@ set(filesToCompile_cpp
     ${repoDir}/src/widgets/SelectionControl.cpp
     ${repoDir}/src/widgets/SmartAmountSettings.cpp
     ${repoDir}/src/widgets/SmartAmounts.cpp
-    ${repoDir}/src/widgets/SmartCheckBox.cpp
     ${repoDir}/src/widgets/SmartDigitWidget.cpp
     ${repoDir}/src/widgets/SmartField.cpp
     ${repoDir}/src/widgets/SmartLabel.cpp

src/measurement/UnitSystem.cpp

@@ -1,5 +1,5 @@
 /*======================================================================================================================
- * measurement/UnitSystem.cpp is part of Brewken, and is copyright the following authors 2009-2023:
+ * measurement/UnitSystem.cpp is part of Brewken, and is copyright the following authors 2009-2024:
  *   • Jeff Bailey <[email protected]>
  *   • Matt Young <[email protected]>
  *   • Mik Firestone <[email protected]>
@@ -272,6 +272,13 @@ Measurement::UnitSystem const * Measurement::UnitSystem::getInstanceByUniqueName
 Measurement::UnitSystem const & Measurement::UnitSystem::getInstance(SystemOfMeasurement const systemOfMeasurement,
                                                                      PhysicalQuantity const physicalQuantity) {
    auto systemsForThisPhysicalQuantity = Measurement::UnitSystem::getUnitSystems(physicalQuantity);
+
+   // Per the comment in the header, if there is _only_ one UnitSystem for the given PhysicalQuantity then we return
+   // that, without trying to match SystemOfMeasurement.
+   if (systemsForThisPhysicalQuantity.size() == 1) {
+      return **systemsForThisPhysicalQuantity.begin();
+   }
+
    auto result = std::find_if(
       systemsForThisPhysicalQuantity.begin(),
       systemsForThisPhysicalQuantity.end(),
@@ -281,11 +288,13 @@ Measurement::UnitSystem const & Measurement::UnitSystem::getInstance(SystemOfMea
    );
 
    if (systemsForThisPhysicalQuantity.end() == result) {
-      // It's a coding error if we didn't find a match
+      // At this point, it's a coding error if we didn't find a match
       qCritical() <<
          Q_FUNC_INFO << "Unable to find a UnitSystem for SystemOfMeasurement" <<
          Measurement::getDisplayName(systemOfMeasurement) << "and PhysicalQuantity" <<
-         Measurement::physicalQuantityStringMapping[physicalQuantity];
+         Measurement::physicalQuantityStringMapping[physicalQuantity] << "(Searched" <<
+         systemsForThisPhysicalQuantity.size() << "option(s).)";
+      qCritical().noquote() << Q_FUNC_INFO << "Stacktrace:" << Logging::getStackTrace();
       Q_ASSERT(false); // Stop here on a debug build
    }
 
@@ -347,8 +356,9 @@ template QTextStream & operator<<(QTextStream & stream, Measurement::UnitSystem:
 //---------------------------------------------------------------------------------------------------------------------
 namespace Measurement::UnitSystems {
    //
-   // NB: For the mass_Xxxx and volume_Xxxx unit systems, to make Measurement::MixedPhysicalQuantities work, we rely on
-   //     them sharing systemOfMeasurement. TODO: This will need to change for PhysicalQuantity::Count
+   // NB: For the mass_Xxxx and volume_Xxxx unit systems, to make PhysicalQuantity::ChoiceOfPhysicalQuantity work, we
+   //     rely on them sharing systemOfMeasurement (Imperial, UsCustomary or Metric).  However, this trick can't work
+   //     with PhysicalQuantity::Count, so there is special handling for that.
    //
    UnitSystem const mass_Metric{PhysicalQuantity::Mass,
                                 &Measurement::Units::kilograms,

src/measurement/UnitSystem.h

@@ -1,5 +1,5 @@
 /*======================================================================================================================
- * measurement/UnitSystem.h is part of Brewken, and is copyright the following authors 2009-2023:
+ * measurement/UnitSystem.h is part of Brewken, and is copyright the following authors 2009-2024:
  *   • Jeff Bailey <[email protected]>
  *   • Matt Young <[email protected]>
  *   • Mik Firestone <[email protected]>
@@ -210,6 +210,14 @@ namespace Measurement {
        */
       static UnitSystem const * getInstanceByUniqueName(QString const & name);
 
+      /**
+       * \brief For the given \c SystemOfMeasurement (eg Metric) and \c PhysicalQuantity (eg Volume), returns the
+       *        appropriate \c UnitSystem (eg volume_Metric).  HOWEVER, if there is no match and the supplied
+       *        \c PhysicalQuantity only has one \c UnitSystem, then return that.  (This allows
+       *        \c ChoiceOfPhysicalQuantity::Mass_Volume_Count to work for the \c Count case.  For Mass and Volume, we
+       *        allow Imperial / UsCustomary / Metric systems of measurement, but it would be madness to have Metric,
+       *        Imperial, etc versions of Count.  So it's better to do some special case handling here.
+       */
       static UnitSystem const & getInstance(SystemOfMeasurement const systemOfMeasurement,
                                             PhysicalQuantity const physicalQuantity);
 

src/widgets/SmartAmountSettings.cpp

@@ -1,5 +1,5 @@
 /*======================================================================================================================
- * widgets/SmartAmountSettings.cpp is part of Brewken, and is copyright the following authors 2023:
+ * widgets/SmartAmountSettings.cpp is part of Brewken, and is copyright the following authors 2023-2024:
  *   • Matt Young <[email protected]>
  *
  * Brewken is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License
@@ -133,12 +133,6 @@ SmartAmounts::ScaleInfo SmartAmountSettings::getScaleInfo() const {
                                      ConvertToPhysicalQuantities(*this->pimpl->m_typeInfo.fieldType));
 }
 
-Measurement::UnitSystem const & SmartAmountSettings::getUnitSystem(SmartAmounts::ScaleInfo const & scaleInfo) const {
-   // It's a coding error to call this for a NonPhysicalQuantity
-   Q_ASSERT(this->pimpl->m_currentPhysicalQuantity);
-   return Measurement::UnitSystem::getInstance(scaleInfo.systemOfMeasurement, *this->pimpl->m_currentPhysicalQuantity);
-}
-
 Measurement::UnitSystem const & SmartAmountSettings::getDisplayUnitSystem() const {
    // It's a coding error to call this for NonPhysicalQuantity, and we assert we never have a ChoiceOfPhysicalQuantity
    // for a SmartLabel that has no associated SmartField.
@@ -171,20 +165,6 @@ void SmartAmountSettings::selectPhysicalQuantity(Measurement::PhysicalQuantity c
    return;
 }
 
-// TODO: We need to rethink this for the case where there are 3 options
-void SmartAmountSettings::selectPhysicalQuantity(bool const isFirst) {
-   // It's a coding error to call this for NonPhysicalQuantity
-   Q_ASSERT(!std::holds_alternative<NonPhysicalQuantity>(*this->pimpl->m_typeInfo.fieldType));
-
-   // It's a coding error to call this if we only hold one PhysicalQuantity
-   Q_ASSERT(!std::holds_alternative<Measurement::PhysicalQuantity>(*this->pimpl->m_typeInfo.fieldType));
-
-   auto const choiceOfPhysicalQuantity = std::get<Measurement::ChoiceOfPhysicalQuantity>(*this->pimpl->m_typeInfo.fieldType);
-   auto const & possibilities = Measurement::allPossibilities(choiceOfPhysicalQuantity);
-   this->pimpl->m_currentPhysicalQuantity = isFirst ? possibilities[0] : possibilities[1];
-   return;
-}
-
 [[nodiscard]] QString SmartAmountSettings::displayAmount(double quantity, unsigned int precision) const {
    // It's a coding error to call this for NonPhysicalQuantity
    Q_ASSERT(!std::holds_alternative<NonPhysicalQuantity>(*this->pimpl->m_typeInfo.fieldType));

src/widgets/SmartAmountSettings.h

@@ -1,5 +1,5 @@
 /*======================================================================================================================
- * widgets/SmartAmountSettings.h is part of Brewken, and is copyright the following authors 2023:
+ * widgets/SmartAmountSettings.h is part of Brewken, and is copyright the following authors 2023-2024:
  *   • Matt Young <[email protected]>
  *
  * Brewken is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License
@@ -54,8 +54,6 @@ class SmartAmountSettings {
     */
    SmartAmounts::ScaleInfo getScaleInfo() const;
 
-   Measurement::UnitSystem const & getUnitSystem(SmartAmounts::ScaleInfo const & scaleInfo) const;
-
    /**
     * \brief Returns the \c UnitSystem that should be used to display this field, based on the forced
     *        \c SystemOfMeasurement for the field if there is one or otherwise on the the system-wide default
@@ -64,34 +62,24 @@ class SmartAmountSettings {
    Measurement::UnitSystem const & getDisplayUnitSystem() const;
 
    /**
-    * \brief Returns what type of field this is - except that, if it is \c Mixed2PhysicalQuantities, will one of the two
-    *        possible \c Measurement::PhysicalQuantity values depending on the value of \c this->units.
+    * \brief Returns what type of field this is - except that, if it is \c Measurement::ChoiceOfPhysicalQuantity, will
+    *        one of the two or three possible \c Measurement::PhysicalQuantity values depending on the value of
+    *        \c this->units.
     *
-    *        It is a coding error to call this function if our field type \c is \c NonPhysicalQuantity.)
+    *        It is a coding error to call this function if our field type is \c NonPhysicalQuantity.)
     */
    Measurement::PhysicalQuantity getPhysicalQuantity() const;
 
    /**
     * \brief If the \c Measurement::PhysicalQuantities supplied in the \c init call was not a single
     *        \c Measurement::PhysicalQuantity, then this member function permits selecting the current
-    *        \c Measurement::PhysicalQuantity from two in the \c Measurement::Mixed2PhysicalQuantities supplied in the
-    *        constructor.
+    *        \c Measurement::PhysicalQuantity from two or three in the \c Measurement::ChoiceOfPhysicalQuantity supplied
+    *        in the constructor.
     *
     *        NB: Caller's responsibility to ensure the display gets updated.  (SmartBase handles this.)
     */
    void selectPhysicalQuantity(Measurement::PhysicalQuantity const physicalQuantity);
 
-   /**
-    * \brief Alternative version of \c selectPhysicalQuantity for generic usage.  By convention, whenever we have a
-    *        checkbox for "Amount is weight?" or "Amount is mass concentration?", \c true (ie box checked) is selecting
-    *        the first of the two values in the \c Mixed2PhysicalQuantities pair (eg \c Mass in \c PqEitherMassOrVolume
-    *        or \c MassConcentration in \c PqEitherMassOrVolumeConcentration).  So, passing in the boolean state of the
-    *        checkbox to this function selects the correct option.
-    *
-    *        NB: Caller's responsibility to ensure the display gets updated.  (SmartBase handles this.)
-    */
-   void selectPhysicalQuantity(bool const isFirst);
-
    /**
     * \brief Use this when you want to do something with the returned QString
     *

src/widgets/SmartBase.h

@@ -1,5 +1,5 @@
 /*======================================================================================================================
- * widgets/SmartBase.h is part of Brewken, and is copyright the following authors 2023:
+ * widgets/SmartBase.h is part of Brewken, and is copyright the following authors 2023-2024:
  *   • Matt Young <[email protected]>
  *
  * Brewken is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License
@@ -17,6 +17,7 @@
 #define WIDGETS_SMARTBASE_H
 #pragma once
 
+#include "utils/CuriouslyRecurringTemplateBase.h"
 #include "widgets/SmartAmountSettings.h"
 
 /**
@@ -26,88 +27,81 @@
  *        \c SmartLabel and \c SmartField.  See comment in \c widgets/SmartField.h for more details.
  *
  *        Derived classes need to implement:
- *           SmartAmountSettings & settings()
+ *           SmartAmountSettings const & settings() const
  *           void correctEnteredText(SmartAmounts::ScaleInfo previousScaleInfo);
  *
+ *        We also need a non-const version of the first of the functions above, but we implement that here in the base
+ *        class.
+ *
  *        At some point we might eliminate this class as it does not add a huge amount, but it was quite useful when I
  *        was refactoring duplicated code out of \c SmartLabel and \c SmartField!
  */
 template<class Derived>
-class SmartBase {
+class SmartBase : public CuriouslyRecurringTemplateBase<SmartBase, Derived> {
 public:
-   SmartBase() :
-      m_derived{static_cast<Derived *>(this)} {
+   SmartBase() {
       return;
    }
    virtual ~SmartBase() = default;
 
+   //! Name-hiding means we cannot call this settings(), so we choose a different name
+   SmartAmountSettings & mutableSettings() {
+      // It's always safe to cast this _to_ const
+      Derived const & constSelf{const_cast<Derived const &>(this->derived())};
+      SmartAmountSettings const & constSettings{constSelf.settings()};
+      // We're casting away constness of the reference, which is a bit less "good practice", but shouldn't break
+      // anything...
+      return const_cast<SmartAmountSettings &>(constSettings);
+   }
+
    TypeInfo const & getTypeInfo() const {
-      return this->m_derived->settings().getTypeInfo();
+      return this->derived().settings().getTypeInfo();
    }
 
    void setForcedSystemOfMeasurement(std::optional<Measurement::SystemOfMeasurement> systemOfMeasurement) {
-      this->m_derived->settings().setForcedSystemOfMeasurement(systemOfMeasurement);
+      this->mutableSettings().setForcedSystemOfMeasurement(systemOfMeasurement);
       return;
    }
 
    void setForcedRelativeScale(std::optional<Measurement::UnitSystem::RelativeScale> relativeScale) {
-      this->m_derived->settings().setForcedRelativeScale(relativeScale);
+      this->mutableSettings().setForcedRelativeScale(relativeScale);
       return;
    }
 
    std::optional<Measurement::SystemOfMeasurement> getForcedSystemOfMeasurement() const {
-      return this->m_derived->settings().getForcedSystemOfMeasurement();
+      return this->derived().settings().getForcedSystemOfMeasurement();
    }
 
    std::optional<Measurement::UnitSystem::RelativeScale> getForcedRelativeScale() const {
-      return this->m_derived->settings().getForcedRelativeScale();
+      return this->derived().settings().getForcedRelativeScale();
    }
 
    SmartAmounts::ScaleInfo getScaleInfo() const {
-      return this->m_derived->settings().getScaleInfo();
-   }
-
-   Measurement::UnitSystem const & getUnitSystem(SmartAmounts::ScaleInfo const & scaleInfo) const {
-      return this->m_derived->settings().getUnitSystem(scaleInfo);
+      return this->derived().settings().getScaleInfo();
    }
 
    Measurement::UnitSystem const & getDisplayUnitSystem() const {
-      return this->m_derived->settings().getDisplayUnitSystem();
+      return this->derived().settings().getDisplayUnitSystem();
    }
 
-
    Measurement::PhysicalQuantity getPhysicalQuantity() const {
-      return this->m_derived->settings().getPhysicalQuantity();
+      return this->derived().settings().getPhysicalQuantity();
    }
 
    void selectPhysicalQuantity(Measurement::PhysicalQuantity const physicalQuantity) {
-      auto const previousScaleInfo = this->m_derived->getScaleInfo();
-      this->m_derived->settings().selectPhysicalQuantity(physicalQuantity);
-      this->m_derived->correctEnteredText(previousScaleInfo);
-      return;
-   }
-
-   [[deprecated]] void selectPhysicalQuantity(bool const isFirst) {
-      auto const previousScaleInfo = this->m_derived->getScaleInfo();
-      this->m_derived->settings().selectPhysicalQuantity(isFirst);
-      this->m_derived->correctEnteredText(previousScaleInfo);
+      auto const previousScaleInfo = this->derived().getScaleInfo();
+      this->mutableSettings().selectPhysicalQuantity(physicalQuantity);
+      this->derived().correctEnteredText(previousScaleInfo);
       return;
    }
 
    [[nodiscard]] QString displayAmount(double quantity, unsigned int precision) const {
-      return this->m_derived->settings().displayAmount(quantity, precision);
+      return this->derived().settings().displayAmount(quantity, precision);
    }
 
    [[nodiscard]] QString displayAmount(Measurement::Amount const & amount, unsigned int precision) {
-      return this->m_derived->settings().displayAmount(amount, precision);
+      return this->mutableSettings().displayAmount(amount, precision);
    }
-
-protected:
-   /**
-    * \brief This is the 'this' pointer downcast to the derived class, which allows us to call non-virtual member
-    *        functions in the derived class from this templated base class.
-    */
-   Derived * m_derived;
-
 };
+
 #endif