diff --git a/.github/workflows/linux-ubuntu.yml b/.github/workflows/linux-ubuntu.yml index 8c21db39e..7aa48ba14 100644 --- a/.github/workflows/linux-ubuntu.yml +++ b/.github/workflows/linux-ubuntu.yml @@ -63,7 +63,7 @@ jobs: # have only Python 3.6.7 and Ubuntu 20.04 only have Python 3.8.2. However Ubuntu 22.04 has Python 3.10.6.) There # are ways to get around this, but, in this context, it's simplest to use a canned GitHub action. # - - uses: actions/setup-python@v4 + - uses: actions/setup-python@v5 with: python-version: '3.10' - name: Install Frameworks and Libraries, and set up Meson build environment @@ -88,6 +88,7 @@ jobs: $GITHUB_WORKSPACE - name: Build (with Meson) + id: meson-build working-directory: ${{github.workspace}}/mbuild shell: bash run: | @@ -98,6 +99,7 @@ jobs: # The 'export QT_QPA_PLATFORM=offscreen' stops Qt's xcb sub-module trying to connect to a non-existent display # (which would cause the test runner to abort before running any tests). - name: Test (via Meson) + id: meson-test working-directory: ${{github.workspace}}/mbuild shell: bash run: | @@ -106,12 +108,14 @@ jobs: meson test - name: Build (with CMake) + id: cmake-build working-directory: ${{github.workspace}}/build shell: bash run: | make - name: Test (via CMake) + id: cmake-test working-directory: ${{github.workspace}}/build shell: bash env: @@ -143,20 +147,20 @@ jobs: mbuild/packages/linux/brewtarget*.rpm.sha256sum retention-days: 7 - - name: Recover Debris Artifacts (CMake) - if: ${{ failure() }} + - name: Recover Debris Artifacts (Meson) + if: ${{ failure() && (steps.meson-build.conclusion == 'failure' || steps.meson-test.conclusion == 'failure') }} uses: actions/upload-artifact@v4 with: - name: build-results-${{matrix.os}} - path: build + name: mbuild-results-${{matrix.os}} + path: mbuild retention-days: 1 - - name: Recover Debris Artifacts (Meson) - if: ${{ failure() }} + - name: Recover Debris Artifacts (CMake) + if: ${{ failure() && (steps.cmake-build.conclusion == 'failure' || steps.cmake-test.conclusion == 'failure') }} uses: actions/upload-artifact@v4 with: - name: mbuild-results-${{matrix.os}} - path: mbuild + name: build-results-${{matrix.os}} + path: build retention-days: 1 # Meson test doesn't show log output on the terminal, but puts it straight to a log file. We don't want to have diff --git a/CHANGES.markdown b/CHANGES.markdown index 10355af83..aaccf0a29 100644 --- a/CHANGES.markdown +++ b/CHANGES.markdown @@ -20,10 +20,12 @@ Bug fixes for the 4.0.5 release (ie bugs in 4.0.5 are fixed in this 4.0.6 releas * None ### Bug Fixes +* Can't add fermentation steps [831](https://github.com/Brewtarget/brewtarget/issues/831) * Ingredient inventory edits not saved [832](https://github.com/Brewtarget/brewtarget/issues/832) * Unsatisfied dependency for Brewtarget update in ubuntu 24.01 [840](https://github.com/Brewtarget/brewtarget/issues/840) * Cmake error on Linux Mint 22 Wilma [843](https://github.com/Brewtarget/brewtarget/issues/843) * Binaries are not signed on Windows [827](https://github.com/Brewtarget/brewtarget/issues/827) +* Adding Mash Step causes core dump [847](https://github.com/Brewtarget/brewtarget/issues/847) ### Release Timestamp Wed, 2 Oct 2024 04:00:06 +0100 diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp index 1c4c551d8..731896be5 100644 --- a/src/MainWindow.cpp +++ b/src/MainWindow.cpp @@ -535,29 +535,6 @@ class MainWindow::impl { return; } - template - void addStepToStepOwner(std::shared_ptr stepOwner, std::shared_ptr step) { - qDebug() << Q_FUNC_INFO; - // - // Mash/Boil/Fermentation Steps are a bit different from most other NamedEntity objects in that they don't really - // have an independent existence. Taking Mash as an example, if you ask a Mash to remove a MashStep then it will - // also tell the ObjectStore to delete it, but, when we're adding a MashStep to a Mash it's easier (for eg the - // implementation of undo/redo) if we add it to the ObjectStore before we call Mash::addMashStep(). - // - ObjectStoreWrapper::insert(step); - this->m_self.doOrRedoUpdate( - newUndoableAddOrRemove(*stepOwner, - &StepOwnerClass::addStep, - step, - &StepOwnerClass::removeStep, - tr("Add %1 step to recipe").arg(StepOwnerClass::localisedName())) - ); - // We don't need to call this->pimpl->m_mashStepTableModel->addMashStep(mashStep) etc here because the change to - // the mash/boil/ferementation will already have triggered the necessary updates to - // this->pimpl->m_mashStepTableModel/this->pimpl->m_boilStepTableModel/etc. - return; - } - // template auto & getStepTableModel() const; // template T> auto & getTableModel() const { return *this-> m_mashStepTableModel; } // template T> auto & getTableModel() const { return *this-> m_boilStepTableModel; } @@ -585,18 +562,21 @@ class MainWindow::impl { void setStepOwner(std::shared_ptr stepOwner) { this->m_recipeObs->setMash(stepOwner); this->m_mashStepTableModel->setMash(stepOwner); + this->m_mashStepEditor->setStepOwner(stepOwner); this->m_self.mashButton->setMash(stepOwner); return; } void setStepOwner(std::shared_ptr stepOwner) { this->m_recipeObs->setBoil(stepOwner); this->m_boilStepTableModel->setBoil(stepOwner); + this->m_boilStepEditor->setStepOwner(stepOwner); this->m_self.boilButton->setBoil(stepOwner); return; } void setStepOwner(std::shared_ptr stepOwner) { this->m_recipeObs->setFermentation(stepOwner); this->m_fermentationStepTableModel->setFermentation(stepOwner); + this->m_fermentationStepEditor->setStepOwner(stepOwner); this->m_self.fermentationButton->setFermentation(stepOwner); return; } @@ -616,13 +596,17 @@ class MainWindow::impl { std::shared_ptr stepOwner = this->m_recipeObs->get(); - if (!stepOwner) { + if (stepOwner) { + // This seems a bit circular, but guarantees that the editor knows to which step owner (eg Mash) the new step + // (eg MashStep) should be added. + this->setStepOwner(stepOwner); + } else { auto defaultStepOwner = std::make_shared(); ObjectStoreWrapper::insert(defaultStepOwner); this->setStepOwner(defaultStepOwner); } - // This ultimately gets stored in MainWindow::addMashStepToMash() etc + // This ultimately gets stored in MainWindow::addStepToStepOwner() etc auto step = std::make_shared(""); this->showStepEditor(step); return; @@ -2360,16 +2344,50 @@ void MainWindow::removeSelectedYeastAddition() { return; } -void MainWindow::addMashStepToMash(std::shared_ptr mashStep) { - this->pimpl->addStepToStepOwner(this->pimpl->m_recipeObs->mash(), mashStep); + +template +void MainWindow::addStepToStepOwner(StepOwnerClass & stepOwner, std::shared_ptr step) { + qDebug() << Q_FUNC_INFO; + // + // Mash/Boil/Fermentation Steps are a bit different from most other NamedEntity objects in that they don't really + // have an independent existence. Taking Mash as an example, if you ask a Mash to remove a MashStep then it will + // also tell the ObjectStore to delete it, but, when we're adding a MashStep to a Mash it's easier (for eg the + // implementation of undo/redo) if we add it to the ObjectStore before we call Mash::addMashStep(). + // + // However, normally, at this point, the new step will already have been added to the DB by + // EditorBase::doSaveAndClose. So we are just belt-and-braces here checking whether it needs to be added. + // + if (step->key() < 0) { + qWarning() << Q_FUNC_INFO << step->metaObject()->className() << "unexpectedly not in DB, so inserting it now."; + ObjectStoreWrapper::insert(step); + } + this->doOrRedoUpdate( + newUndoableAddOrRemove(stepOwner, + &StepOwnerClass::addStep, + step, + &StepOwnerClass::removeStep, + tr("Add %1 step to recipe").arg(StepOwnerClass::localisedName())) + ); + // We don't need to call this->pimpl->m_mashStepTableModel->addMashStep(mashStep) etc here because the change to + // the mash/boil/ferementation will already have triggered the necessary updates to + // this->pimpl->m_mashStepTableModel/this->pimpl->m_boilStepTableModel/etc. + return; +} + +template +void MainWindow::addStepToStepOwner(std::shared_ptr stepOwner, std::shared_ptr step) { + this->addStepToStepOwner(*stepOwner, step); +} +void MainWindow::addStepToStepOwner(std::shared_ptr mashStep) { + this->addStepToStepOwner(this->pimpl->m_recipeObs->mash(), mashStep); return; } -void MainWindow::addBoilStepToBoil(std::shared_ptr boilStep) { - this->pimpl->addStepToStepOwner(this->pimpl->m_recipeObs->boil(), boilStep); +void MainWindow::addStepToStepOwner(std::shared_ptr boilStep) { + this->addStepToStepOwner(this->pimpl->m_recipeObs->boil(), boilStep); return; } -void MainWindow::addFermentationStepToFermentation(std::shared_ptr fermentationStep) { - this->pimpl->addStepToStepOwner(this->pimpl->m_recipeObs->fermentation(), fermentationStep); +void MainWindow::addStepToStepOwner(std::shared_ptr fermentationStep) { + this->addStepToStepOwner(this->pimpl->m_recipeObs->fermentation(), fermentationStep); return; } diff --git a/src/MainWindow.h b/src/MainWindow.h index 67ba95ead..a13c48a89 100644 --- a/src/MainWindow.h +++ b/src/MainWindow.h @@ -126,6 +126,15 @@ class MainWindow : public QMainWindow, public Ui::mainWindow { */ template void addIngredientToRecipe(NE & ne); + //! \brief Actually add the new mash step to (the mash of) the recipe (in an undoable way). + template + void addStepToStepOwner(StepOwnerClass & stepOwner, std::shared_ptr step); + template + void addStepToStepOwner(std::shared_ptr stepOwner, std::shared_ptr step); + void addStepToStepOwner(std::shared_ptr step); + void addStepToStepOwner(std::shared_ptr step); + void addStepToStepOwner(std::shared_ptr step); + public slots: //! \brief Accepts Recipe changes, and takes appropriate action to show the changes. @@ -195,11 +204,6 @@ public slots: void addBoilStep(); void addFermentationStep(); - //! \brief Actually add the new mash step to (the mash of) the recipe (in an undoable way). - void addMashStepToMash (std::shared_ptr step); - void addBoilStepToBoil (std::shared_ptr step); - void addFermentationStepToFermentation(std::shared_ptr step); - //! \brief Move currently selected mash step down. void moveSelectedMashStepUp(); void moveSelectedBoilStepUp(); diff --git a/src/database/ObjectStore.cpp b/src/database/ObjectStore.cpp index d4ad84b54..410ae2585 100644 --- a/src/database/ObjectStore.cpp +++ b/src/database/ObjectStore.cpp @@ -1183,6 +1183,8 @@ class ObjectStore::impl { qDebug() << Q_FUNC_INFO << "Inserting" << object.metaObject()->className() << "main table row with database query " << queryString; + // Uncomment the following to track down errors where we're trying to insert an object to the database twice +// qDebug().noquote() << Q_FUNC_INFO << Logging::getStackTrace(); // // Bind the values diff --git a/src/editors/BoilStepEditor.cpp b/src/editors/BoilStepEditor.cpp index f4d4899cc..eddd5e8d4 100755 --- a/src/editors/BoilStepEditor.cpp +++ b/src/editors/BoilStepEditor.cpp @@ -20,6 +20,7 @@ BoilStepEditor::BoilStepEditor(QWidget* parent, QString const editorName) : QDialog{parent}, + StepEditorBase{}, EditorBase(editorName) { this->setupUi(this); this->postSetupUiInit({ diff --git a/src/editors/BoilStepEditor.h b/src/editors/BoilStepEditor.h index 87c595ce9..459135ab2 100755 --- a/src/editors/BoilStepEditor.h +++ b/src/editors/BoilStepEditor.h @@ -32,6 +32,7 @@ */ class BoilStepEditor : public QDialog, public Ui::boilStepEditor, + public StepEditorBase, public EditorBase { Q_OBJECT diff --git a/src/editors/EditorBase.h b/src/editors/EditorBase.h index 7b216d002..d12dcc2a0 100755 --- a/src/editors/EditorBase.h +++ b/src/editors/EditorBase.h @@ -17,6 +17,7 @@ #define EDITORS_EDITORBASE_H #pragma once +#include #include #include #include @@ -29,10 +30,52 @@ #include "BtHorizontalTabs.h" #include "database/ObjectStoreWrapper.h" #include "editors/EditorBaseField.h" +#include "MainWindow.h" #include "model/NamedEntity.h" #include "model/Recipe.h" // Need to include this this to be able to cast Recipe to QObject #include "utils/CuriouslyRecurringTemplateBase.h" +/** + * \brief Extra base class for \c MashStepEditor, \c BoilStepEditor and \c FermentationStepEditor to inherit from + * \b before inheriting from \c EditorBase. This provides the functionality that, if we are creating a new step, + * rather than editing an existing one, then we need to be able to set the owner (ie the \c Mash, \c Boil or + * \c Fermentation) when we click \b Save, because the steps have no independent existence without their owner. + */ +template class StepEditorPhantom; +template +class StepEditorBase : public CuriouslyRecurringTemplateBase { +public: + StepEditorBase() : + m_stepOwner{nullptr} { + return; + } + + void setStepOwner(std::shared_ptr stepOwner) { + this->m_stepOwner = stepOwner; + return; + } + + /** + * \brief If the step being edited is new, then, when save is clicked, this should be called + */ + void addStepToStepOwner(std::shared_ptr step) { + // + // Going via MainWindow makes the action undoable + // + Q_ASSERT(this->m_stepOwner); + MainWindow::instance().addStepToStepOwner(*this->m_stepOwner, step); + return; + } + + /** + * \brief I tried putting this in \c EditorBase and having \c NE::OwnerClass eavluated only when it is valid (ie + * when HasStepOwner is true), and some replaced with some dummy type otherwise. This + * seems quite to do. Eg \c std::conditional requires both its class parameters to be valid. So an extra + * CRTP base class was born. + */ + std::shared_ptr m_stepOwner; +}; + /** * \brief This is used as a template parameter to turn on and off various \b small features in \c EditorBase (in * conjunction with the concepts defined below). @@ -337,6 +380,16 @@ class EditorBase : public CuriouslyRecurringTemplateBase return true; } + //! No-op version + void updateStepOwnerIfNeeded() requires (!std::is_base_of, Derived>::value) { + return; + } + //! Substantive version + void updateStepOwnerIfNeeded() requires (std::is_base_of, Derived>::value) { + this->derived().addStepToStepOwner(this->m_editItem); + return; + } + /** * \brief Subclass should call this from its \c save slot */ @@ -357,6 +410,7 @@ class EditorBase : public CuriouslyRecurringTemplateBase ObjectStoreWrapper::insert(this->m_editItem); } this->writeLateFieldsToEditItem(); + this->updateStepOwnerIfNeeded(); this->derived().setVisible(false); return; diff --git a/src/editors/FermentationStepEditor.cpp b/src/editors/FermentationStepEditor.cpp index b80ef2b37..a2072cb20 100755 --- a/src/editors/FermentationStepEditor.cpp +++ b/src/editors/FermentationStepEditor.cpp @@ -20,6 +20,7 @@ FermentationStepEditor::FermentationStepEditor(QWidget* parent, QString const editorName) : QDialog{parent}, + StepEditorBase{}, EditorBase(editorName) { this->setupUi(this); this->postSetupUiInit({ diff --git a/src/editors/FermentationStepEditor.h b/src/editors/FermentationStepEditor.h index 7bd06820c..0a1cd5618 100755 --- a/src/editors/FermentationStepEditor.h +++ b/src/editors/FermentationStepEditor.h @@ -32,6 +32,7 @@ */ class FermentationStepEditor : public QDialog, public Ui::fermentationStepEditor, + public StepEditorBase, public EditorBase { Q_OBJECT diff --git a/src/editors/MashStepEditor.cpp b/src/editors/MashStepEditor.cpp index 97e6457ba..cff3caaf4 100755 --- a/src/editors/MashStepEditor.cpp +++ b/src/editors/MashStepEditor.cpp @@ -23,7 +23,8 @@ MashStepEditor::MashStepEditor(QWidget* parent, QString const editorName) : QDialog{parent}, - EditorBase(editorName) { + StepEditorBase{}, + EditorBase{editorName} { this->setupUi(this); this->postSetupUiInit({ // diff --git a/src/editors/MashStepEditor.h b/src/editors/MashStepEditor.h index 508a9699b..c97dd8877 100755 --- a/src/editors/MashStepEditor.h +++ b/src/editors/MashStepEditor.h @@ -35,6 +35,7 @@ */ class MashStepEditor : public QDialog, public Ui::mashStepEditor, + public StepEditorBase, public EditorBase { Q_OBJECT diff --git a/src/model/BoilStep.cpp b/src/model/BoilStep.cpp index 17fa474d5..bea79002f 100755 --- a/src/model/BoilStep.cpp +++ b/src/model/BoilStep.cpp @@ -56,6 +56,7 @@ static_assert(std::is_base_of::value); BoilStep::BoilStep(QString name) : StepExtended {name }, + StepBase{}, m_chillingType{std::nullopt} { CONSTRUCTOR_END @@ -64,11 +65,8 @@ BoilStep::BoilStep(QString name) : BoilStep::BoilStep(NamedParameterBundle const & namedParameterBundle) : StepExtended{namedParameterBundle}, + StepBase{namedParameterBundle}, SET_OPT_ENUM_FROM_NPB(m_chillingType, BoilStep::ChillingType, namedParameterBundle, PropertyNames::BoilStep::chillingType) { - // See comment in Step constructor. We're saying that, if rampTime_mins is present in the bundle (which it won't - // always be because it's optional) then it is supported by this class. In other words, either it's not there, or - // (if it is then) it's supported. - Q_ASSERT(!namedParameterBundle.contains(PropertyNames::Step::rampTime_mins) || this->rampTimeIsSupported()); CONSTRUCTOR_END return; @@ -76,6 +74,7 @@ BoilStep::BoilStep(NamedParameterBundle const & namedParameterBundle) : BoilStep::BoilStep(BoilStep const & other) : StepExtended {other }, + StepBase{other}, m_chillingType{other.m_chillingType} { CONSTRUCTOR_END diff --git a/src/model/BoilStep.h b/src/model/BoilStep.h index 97f0d9a0c..9645c616e 100755 --- a/src/model/BoilStep.h +++ b/src/model/BoilStep.h @@ -32,17 +32,20 @@ AddPropertyName(chillingType) #undef AddPropertyName //=========================================== End of property name constants =========================================== //====================================================================================================================== - +/** + * On \c BoilStep, \c stepTime_mins and \c startTemp_c are optional + */ +#define BoilStepOptions StepBaseOptions{.stepTimeRequired = false, .startTempRequired = false, .rampTimeSupported = true} /** * \class BoilStep is a step in a a boil process. It can be used to support pre-boil steps, non-boiling pasteurisation * steps, boiling, whirlpool steps, and chilling. * * \brief As a \c MashStep is to a \c Mash, so a \c BoilStep is to a \c Boil */ -class BoilStep : public StepExtended, public StepBase { +class BoilStep : public StepExtended, public StepBase { Q_OBJECT - STEP_COMMON_DECL(Boil) + STEP_COMMON_DECL(Boil, BoilStepOptions) public: /** @@ -73,6 +76,12 @@ class BoilStep : public StepExtended, public StepBase { */ static EnumStringMapping const chillingTypeDisplayNames; + // + // This alias makees it easier to template a number of functions that are essentially the same for all subclasses of + // Step. + // + using OwnerClass = Boil; + /** * \brief Mapping of names to types for the Qt properties of this class. See \c NamedEntity::typeLookup for more * info. diff --git a/src/model/FermentationStep.cpp b/src/model/FermentationStep.cpp index 333dcad73..a5479e5fa 100755 --- a/src/model/FermentationStep.cpp +++ b/src/model/FermentationStep.cpp @@ -47,6 +47,7 @@ static_assert(std::is_base_of::value); FermentationStep::FermentationStep(QString name) : StepExtended{name}, + StepBase{}, m_freeRise {std::nullopt}, m_vessel {""} { @@ -55,11 +56,10 @@ FermentationStep::FermentationStep(QString name) : } FermentationStep::FermentationStep(NamedParameterBundle const & namedParameterBundle) : - StepExtended (namedParameterBundle ), + StepExtended {namedParameterBundle }, + StepBase{namedParameterBundle}, SET_REGULAR_FROM_NPB (m_freeRise, namedParameterBundle, PropertyNames::FermentationStep::freeRise, std::nullopt), SET_REGULAR_FROM_NPB (m_vessel , namedParameterBundle, PropertyNames::FermentationStep::vessel , QString() ) { - // See comment in Step constructor - Q_ASSERT(this->rampTimeIsSupported() == namedParameterBundle.contains(PropertyNames::Step::rampTime_mins)); CONSTRUCTOR_END return; @@ -67,6 +67,7 @@ FermentationStep::FermentationStep(NamedParameterBundle const & namedParameterBu FermentationStep::FermentationStep(FermentationStep const & other) : StepExtended{other }, + StepBase{other}, m_freeRise {other.m_freeRise}, m_vessel {other.m_vessel } { @@ -84,23 +85,5 @@ QString FermentationStep::vessel () const { return this->m_vessel void FermentationStep::setFreeRise(std::optional const val) { SET_AND_NOTIFY(PropertyNames::FermentationStep::freeRise, this->m_freeRise, val); return; } void FermentationStep::setVessel (QString const & val) { SET_AND_NOTIFY(PropertyNames::FermentationStep::vessel , this->m_vessel , val); return; } -//============================================ "DON'T USE" MEMBER FUNCTIONS ============================================ -// See related Q_PROPERTY in model/Step.h comment above for why these are virtual. Essentially we inherit them from -// Step but want to assert at runtime that it is a coding error if they are every called. -[[deprecated]] std::optional FermentationStep::rampTime_mins() const { - qCritical().noquote() << Q_FUNC_INFO << "Erroneous function call: " << Logging::getStackTrace(); - Q_ASSERT(false); - return std::nullopt; -} -[[deprecated]] void FermentationStep::setRampTime_mins(std::optional const) { - qCritical().noquote() << Q_FUNC_INFO << "Erroneous function call: " << Logging::getStackTrace(); - Q_ASSERT(false); - return; -} - -//=============================================== OTHER MEMBER FUNCTIONS =============================================== -[[nodiscard]] bool FermentationStep::rampTimeIsSupported() const { return false; } - - // Insert boiler-plate wrapper functions that call down to StepBase STEP_COMMON_CODE(Fermentation) diff --git a/src/model/FermentationStep.h b/src/model/FermentationStep.h index 88078e187..8fdcc3e8a 100755 --- a/src/model/FermentationStep.h +++ b/src/model/FermentationStep.h @@ -33,17 +33,20 @@ AddPropertyName(vessel ) #undef AddPropertyName //=========================================== End of property name constants =========================================== //====================================================================================================================== - +/** + * On \c FermentationStep, \c stepTime_mins and \c startTemp_c are optional, and \c rampTime_mins is not to be used + */ +#define FermentationStepOptions StepBaseOptions{.stepTimeRequired = false, .startTempRequired = false, .rampTimeSupported = false} /** * \class FermentationStep is a step in a a fermentation process. * * \brief As a \c MashStep is to a \c Mash, and a \c BoilStep is to a \c Boil, so a \c FermentationStep is to a * \c Fermentation. */ -class FermentationStep : public StepExtended, public StepBase { +class FermentationStep : public StepExtended, public StepBase { Q_OBJECT - STEP_COMMON_DECL(Fermentation) + STEP_COMMON_DECL(Fermentation, FermentationStepOptions) public: /** @@ -51,6 +54,12 @@ class FermentationStep : public StepExtended, public StepBase const val); void setVessel (QString const & val); - //========================================== "DON'T USE" MEMBER FUNCTIONS =========================================== - // See related Q_PROPERTY in model/Step.h comment above for why these are virtual. Essentially we inherit them from - // Step but want to assert at runtime that it is a coding error if they are every called. - [[deprecated]] virtual std::optional rampTime_mins() const; - [[deprecated]] virtual void setRampTime_mins(std::optional const val); - - //============================================= OTHER MEMBER FUNCTIONS ============================================== - [[nodiscard]] virtual bool rampTimeIsSupported() const; - signals: protected: diff --git a/src/model/MashStep.cpp b/src/model/MashStep.cpp index 5d2658f60..862f4283e 100644 --- a/src/model/MashStep.cpp +++ b/src/model/MashStep.cpp @@ -82,6 +82,9 @@ static_assert(std::is_base_of::value); MashStep::MashStep(QString name) : Step {name}, + StepBase{}, m_type {MashStep::Type::Infusion}, m_amount_l {0.0 }, m_infuseTemp_c {std::nullopt }, @@ -93,16 +96,15 @@ MashStep::MashStep(QString name) : } MashStep::MashStep(NamedParameterBundle const & namedParameterBundle) : - Step (namedParameterBundle ), + Step{namedParameterBundle}, + StepBase{namedParameterBundle}, SET_REGULAR_FROM_NPB (m_type , namedParameterBundle, PropertyNames::MashStep::type ), SET_REGULAR_FROM_NPB (m_amount_l , namedParameterBundle, PropertyNames::MashStep::amount_l , 0.0), SET_REGULAR_FROM_NPB (m_infuseTemp_c , namedParameterBundle, PropertyNames::MashStep::infuseTemp_c ), // ⮜⮜⮜ All below added for BeerJSON support ⮞⮞⮞ SET_REGULAR_FROM_NPB (m_liquorToGristRatio_lKg, namedParameterBundle, PropertyNames::MashStep::liquorToGristRatio_lKg) { - // See comment in Step constructor. We're saying that, if rampTime_mins is present in the bundle (which it won't - // always be because it's optional) then it is supported by this class. In other words, either it's not there, or - // (if it is then) it's supported. - Q_ASSERT(!namedParameterBundle.contains(PropertyNames::Step::rampTime_mins) || this->rampTimeIsSupported()); // // If we were constructed from BeerXML, it will have set decoctionAmount_l or infuseAmount_l instead of amount_l // @@ -120,6 +122,9 @@ MashStep::MashStep(NamedParameterBundle const & namedParameterBundle) : MashStep::MashStep(MashStep const & other) : Step {other}, + StepBase{}, m_type {other.m_type }, m_amount_l {other.m_amount_l }, m_infuseTemp_c {other.m_infuseTemp_c }, @@ -144,10 +149,9 @@ std::optional MashStep::liquorToGristRatio_lKg() const { return this->m_ [[deprecated]] double MashStep::decoctionAmount_l() const { return this->m_amount_l; } //============================================= "SETTER" MEMBER FUNCTIONS ============================================== -void MashStep::setType (MashStep::Type const val) { SET_AND_NOTIFY(PropertyNames::MashStep::type , this->m_type , val ); return; } -void MashStep::setAmount_l (double const val) { SET_AND_NOTIFY(PropertyNames::MashStep::amount_l , this->m_amount_l , val ); return; } -///void MashStep::setStepTemp_c (double const val) { SET_AND_NOTIFY(PropertyNames::MashStep::stepTemp_c , this->m_stepTemp_c , this->enforceMin(val, "step temp", PhysicalConstants::absoluteZero)); return; } -void MashStep::setInfuseTemp_c (std::optional const val) { SET_AND_NOTIFY(PropertyNames::MashStep::infuseTemp_c , this->m_infuseTemp_c , val ); return; } +void MashStep::setType (MashStep::Type const val) { SET_AND_NOTIFY(PropertyNames::MashStep::type , this->m_type , val); return; } +void MashStep::setAmount_l (double const val) { SET_AND_NOTIFY(PropertyNames::MashStep::amount_l , this->m_amount_l , val); return; } +void MashStep::setInfuseTemp_c(std::optional const val) { SET_AND_NOTIFY(PropertyNames::MashStep::infuseTemp_c, this->m_infuseTemp_c, val); return; } // ⮜⮜⮜ All below added for BeerJSON support ⮞⮞⮞ void MashStep::setLiquorToGristRatio_lKg(std::optional const val) { SET_AND_NOTIFY(PropertyNames::MashStep::liquorToGristRatio_lKg, this->m_liquorToGristRatio_lKg, val ); return; } @@ -176,8 +180,5 @@ bool MashStep::isDecoction() const { return (m_type == MashStep::Type::Decoction); } -[[nodiscard]] bool MashStep:: stepTimeIsRequired() const { return true; } -[[nodiscard]] bool MashStep::startTempIsRequired() const { return true; } - // Insert boiler-plate wrapper functions that call down to StepBase STEP_COMMON_CODE(Mash) diff --git a/src/model/MashStep.h b/src/model/MashStep.h index 8a4b6826d..f61eaa265 100644 --- a/src/model/MashStep.h +++ b/src/model/MashStep.h @@ -45,17 +45,19 @@ AddPropertyName(type ) #undef AddPropertyName //=========================================== End of property name constants =========================================== //====================================================================================================================== - - +/** + * On \c MashStep, \c stepTime_mins is required, \c startTemp_c is required + */ +#define MashStepOptions StepBaseOptions{.stepTimeRequired = true, .startTempRequired = true, .rampTimeSupported = true} /*! * \class MashStep * * \brief Model for a mash step record in the database. */ -class MashStep : public Step, public StepBase { +class MashStep : public Step, public StepBase { Q_OBJECT - STEP_COMMON_DECL(Mash) + STEP_COMMON_DECL(Mash, MashStepOptions) public: /** @@ -89,6 +91,12 @@ class MashStep : public Step, public StepBase { */ static EnumStringMapping const typeDisplayNames; + // + // This alias makees it easier to template a number of functions that are essentially the same for all subclasses of + // Step. + // + using OwnerClass = Mash; + /** * \brief Mapping of names to types for the Qt properties of this class. See \c NamedEntity::typeLookup for more * info. @@ -174,10 +182,6 @@ class MashStep : public Step, public StepBase { protected: virtual bool isEqualTo(NamedEntity const & other) const; - //! On \c MashStep, stepTime_mins is required - [[nodiscard]] virtual bool stepTimeIsRequired() const; - //! On \c MashStep, startTemp_c is required - [[nodiscard]] virtual bool startTempIsRequired() const; private: Type m_type ; diff --git a/src/model/NamedEntity.h b/src/model/NamedEntity.h index 90f90f79f..53780bc7b 100644 --- a/src/model/NamedEntity.h +++ b/src/model/NamedEntity.h @@ -83,7 +83,7 @@ AddPropertyName(parentKey) * NOTE that, strictly, this is not needed on abstract classes, though it is relatively harmless to include on * them. */ -#define TYPE_LOOKUP_GETTER inline virtual TypeLookup const & getTypeLookup() const { return typeLookup; } +#define TYPE_LOOKUP_GETTER inline virtual TypeLookup const & getTypeLookup() const override { return typeLookup; } /*! * \class NamedEntity @@ -160,8 +160,9 @@ class NamedEntity : public QObject { * which just returns the relevant object. That's a turn-the-handle function */ static TypeLookup const typeLookup; - // See comment above for what this does - TYPE_LOOKUP_GETTER + // See comment above for what this does. We can't use the macro here because this is the base function that all + // other classes override (so this is the one place the override keyword is not valid. + inline virtual TypeLookup const & getTypeLookup() const { return typeLookup; } NamedEntity(QString t_name, bool t_display = false); NamedEntity(NamedEntity const & other); diff --git a/src/model/Step.cpp b/src/model/Step.cpp index b18885ae6..9421d9b7f 100755 --- a/src/model/Step.cpp +++ b/src/model/Step.cpp @@ -19,23 +19,6 @@ #include "PhysicalConstants.h" #include "utils/AutoCompare.h" -namespace { - // Everything has to be double because our underlying measure (minutes) is allowed to be measured in fractions. - constexpr double minutesInADay = 24.0 * 60.0; - std::optional minutesToDays(std::optional val) { - if (val) { - return *val / minutesInADay; - } - return std::nullopt; - } - std::optional daysToMinutes(std::optional val) { - if (val) { - return *val * minutesInADay; - } - return std::nullopt; - } -} - QString Step::localisedName() { return tr("Step"); } bool Step::isEqualTo(NamedEntity const & other) const { @@ -80,7 +63,7 @@ TypeLookup const Step::typeLookup { //==================================================== CONSTRUCTORS ==================================================== Step::Step(QString name) : NamedEntity {name, true}, - m_stepTime_mins {0.0 }, + m_stepTime_mins {std::nullopt}, m_startTemp_c {std::nullopt}, m_endTemp_c {std::nullopt}, m_stepNumber {0 }, @@ -96,7 +79,7 @@ Step::Step(QString name) : } Step::Step(NamedParameterBundle const & namedParameterBundle) : - NamedEntity (namedParameterBundle ), + NamedEntity (namedParameterBundle), // See below for m_stepTime_mins SET_REGULAR_FROM_NPB (m_startTemp_c , namedParameterBundle, PropertyNames::Step::startTemp_c , std::nullopt), SET_REGULAR_FROM_NPB (m_endTemp_c , namedParameterBundle, PropertyNames::Step:: endTemp_c , std::nullopt), @@ -107,17 +90,10 @@ Step::Step(NamedParameterBundle const & namedParameterBundle) : SET_REGULAR_FROM_NPB (m_rampTime_mins , namedParameterBundle, PropertyNames::Step::rampTime_mins , std::nullopt), SET_REGULAR_FROM_NPB (m_startAcidity_pH, namedParameterBundle, PropertyNames::Step::startAcidity_pH, std::nullopt), SET_REGULAR_FROM_NPB (m_endAcidity_pH , namedParameterBundle, PropertyNames::Step:: endAcidity_pH, std::nullopt) { - // It would be nice to be able to assert here that rampTime_mins is present in the bundle only if the subclass - // supports it. However, we cannot safely call a virtual member function from a base class constructor, so we have - // to do such asserts in the derived classes. - // If we're being constructed from a BeerXML file, we use the property stepTime_days for RECIPE > PRIMARY_AGE etc - // Otherwise we use the stepTime_mins property - if (!SET_IF_PRESENT_FROM_NPB_NO_MV(Step::setStepTime_mins, namedParameterBundle, PropertyNames::Step::stepTime_mins) && - !SET_IF_PRESENT_FROM_NPB_NO_MV(Step::setStepTime_days, namedParameterBundle, PropertyNames::Step::stepTime_days)) { - qWarning() << Q_FUNC_INFO << "Neither stepTime_mins nor stepTime_days set in bundle, so step time will be 0."; - this->m_stepTime_mins = 0.0; - } + // Note that we cannot call this->setStepTime_mins() or this->setStepTime_days() here, as these are virtual functions + // which are not callable until our child class constructor is run. Therefore handling of + // PropertyNames::Step::stepTime_mins and PropertyNames::Step::stepTime_days is done in StepBase CONSTRUCTOR_END return; @@ -143,51 +119,19 @@ Step::Step(Step const & other) : Step::~Step() = default; //============================================= "GETTER" MEMBER FUNCTIONS ============================================== -std::optional Step::stepTime_mins () const { - Q_ASSERT(!this->stepTimeIsRequired() || this->m_stepTime_mins); - return this->m_stepTime_mins; -} -std::optional Step::stepTime_days () const { - return minutesToDays(this->stepTime_mins()); -} -std::optional Step::startTemp_c () const { - Q_ASSERT(!this->startTempIsRequired() || this->m_startTemp_c); - return this->m_startTemp_c; -} std::optional Step::endTemp_c () const { return this->m_endTemp_c ; } int Step::stepNumber () const { return this->m_stepNumber ; } int Step::ownerId () const { return this->m_ownerId ; } // ⮜⮜⮜ All below added for BeerJSON support ⮞⮞⮞ QString Step::description () const { return this->m_description ; } -std::optional Step::rampTime_mins () const { return this->m_rampTime_mins ; } std::optional Step::startAcidity_pH() const { return this->m_startAcidity_pH; } std::optional Step::endAcidity_pH () const { return this->m_endAcidity_pH ; } //============================================= "SETTER" MEMBER FUNCTIONS ============================================== -void Step::setStepTime_mins(std::optional const val) { - Q_ASSERT(!this->stepTimeIsRequired() || val); - SET_AND_NOTIFY(PropertyNames::Step::stepTime_mins, this->m_stepTime_mins, val); - return; -} -void Step::setStepTime_days(std::optional const val) { - this->setStepTime_mins(daysToMinutes(val)); - return; -} -void Step::setStartTemp_c(std::optional const val) { - Q_ASSERT(!this->startTempIsRequired() || val); - SET_AND_NOTIFY(PropertyNames::Step::startTemp_c, this->m_startTemp_c, this->enforceMin(val, "start temp", PhysicalConstants::absoluteZero)); - return; -} -void Step::setEndTemp_c (std::optional const val) { SET_AND_NOTIFY(PropertyNames::Step::endTemp_c , this->m_endTemp_c , this->enforceMin(val, "end temp" , PhysicalConstants::absoluteZero)); return; } -void Step::setStepNumber (int const val) { SET_AND_NOTIFY(PropertyNames::Step::stepNumber , this->m_stepNumber , val ); return; } -void Step::setOwnerId (int const val) { this->m_ownerId = val; this->propagatePropertyChange(PropertyNames::Step::ownerId, false); return; } -// ⮜⮜⮜ All below added for BeerJSON support ⮞⮞⮞ -void Step::setDescription (QString const & val) { SET_AND_NOTIFY(PropertyNames::Step::description , this->m_description , val ); return; } -void Step::setRampTime_mins (std::optional const val) { SET_AND_NOTIFY(PropertyNames::Step::rampTime_mins , this->m_rampTime_mins , val ); return; } -void Step::setStartAcidity_pH (std::optional const val) { SET_AND_NOTIFY(PropertyNames::Step::startAcidity_pH, this->m_startAcidity_pH, val ); return; } -void Step::setEndAcidity_pH (std::optional const val) { SET_AND_NOTIFY(PropertyNames::Step::endAcidity_pH , this->m_endAcidity_pH , val ); return; } - -//=============================================== OTHER MEMBER FUNCTIONS =============================================== -[[nodiscard]] bool Step:: stepTimeIsRequired() const { return false; } -[[nodiscard]] bool Step::startTempIsRequired() const { return false; } -[[nodiscard]] bool Step::rampTimeIsSupported() const { return true; } +void Step::setEndTemp_c (std::optional const val) { SET_AND_NOTIFY(PropertyNames::Step::endTemp_c , this->m_endTemp_c , this->enforceMin(val, "end temp" , PhysicalConstants::absoluteZero)); return; } +void Step::setStepNumber (int const val) { SET_AND_NOTIFY(PropertyNames::Step::stepNumber , this->m_stepNumber , val); return; } +void Step::setOwnerId (int const val) { this->m_ownerId = val; this->propagatePropertyChange(PropertyNames::Step::ownerId, false); return; } +// ⮜⮜⮜ All below added for Besupport ⮞⮞⮞ +void Step::setDescription (QString const & val) { SET_AND_NOTIFY(PropertyNames::Step::description , this->m_description , val); return; } +void Step::setStartAcidity_pH(std::optional const val) { SET_AND_NOTIFY(PropertyNames::Step::startAcidity_pH, this->m_startAcidity_pH, val); return; } +void Step::setEndAcidity_pH (std::optional const val) { SET_AND_NOTIFY(PropertyNames::Step::endAcidity_pH , this->m_endAcidity_pH , val); return; } diff --git a/src/model/Step.h b/src/model/Step.h index 777521588..8dd43ceed 100755 --- a/src/model/Step.h +++ b/src/model/Step.h @@ -37,7 +37,7 @@ AddPropertyName(rampTime_mins ) AddPropertyName(startAcidity_pH) AddPropertyName(startTemp_c ) AddPropertyName(stepNumber ) -AddPropertyName(stepTime_days ) +AddPropertyName(stepTime_days ) // Mostly needed for BeerXML AddPropertyName(stepTime_mins ) #undef AddPropertyName //=========================================== End of property name constants =========================================== @@ -45,7 +45,7 @@ AddPropertyName(stepTime_mins ) /** - * \brief Common base class for \c MashStep, \c BoilStep, \c FermentationStep + * \brief Common abstract base class for \c MashStep, \c BoilStep, \c FermentationStep * * In BeerJSON, the step types have overlapping sets of fields, which correspond to our properties as follows * (where ‡ means a field is required): @@ -109,7 +109,8 @@ Q_OBJECT /** * \brief The time of the step in min. * NOTE: This is required for MashStep but optional for BoilStep and FermentationStep. We make it optional - * here but classes that need it required should override the virtual function \c stepTimeIsRequired + * here but classes that need it required should set \c StepBaseOptions.stepTimeIsRequired parameter on + * \c StepBase template. See \c StepBase for getters and setters. */ Q_PROPERTY(std::optional stepTime_mins READ stepTime_mins WRITE setStepTime_mins ) /** @@ -123,7 +124,8 @@ Q_OBJECT * dealing with the mash step temperature. * * NOTE: This is required for MashStep but optional for BoilStep and FermentationStep. We make it optional - * here but classes that need it required should override the virtual function \c startTempIsRequired + * here but classes that need it required should set \c StepBaseOptions.startTempIsRequired parameter on + * \c StepBase template. See \c StepBase for getters and setters. */ Q_PROPERTY(std::optional startTemp_c READ startTemp_c WRITE setStartTemp_c ) /** @@ -146,9 +148,10 @@ Q_OBJECT * * NOTE: This property is \b not used by \c FermentationStep. (It is the only property shared by \c MashStep * and \c BoilStep that is not also needed in \c FermentationStep. We can't really do mix-ins in Qt, so - * it's simplest just to not use it in \c FermentationStep. We make the getters and setters virtual so - * we can at least get a run-time error if we accidentally try to use this property on a - * \c FermentationStep.) + * it's simplest just to not use it in \c FermentationStep. We require the classes that use this + * property to set set \c StepBaseOptions.rampTimeIsSupported parameter on \c StepBase template, so we + * can at least get a run-time error if we accidentally try to use this property on a + * \c FermentationStep.) See \c StepBase for getters and setters. */ Q_PROPERTY(std::optional rampTime_mins READ rampTime_mins WRITE setRampTime_mins ) //! \brief The step number in a sequence of other steps. Step numbers start from 1. @@ -161,28 +164,34 @@ Q_OBJECT Q_PROPERTY(std::optional endAcidity_pH READ endAcidity_pH WRITE setEndAcidity_pH ) //============================================ "GETTER" MEMBER FUNCTIONS ============================================ - std::optional stepTime_mins () const; - std::optional stepTime_days () const; - std::optional startTemp_c () const; + std::optional endTemp_c () const; int stepNumber () const; int ownerId () const; // ⮜⮜⮜ All below added for BeerJSON support ⮞⮞⮞ QString description () const; - virtual std::optional rampTime_mins () const; // See related Q_PROPERTY comment above for why this is virtual std::optional startAcidity_pH() const; std::optional endAcidity_pH() const; + // See model/StepBase.h for overrides of these + virtual std::optional stepTime_mins() const = 0; + virtual std::optional stepTime_days() const = 0; + virtual std::optional startTemp_c () const = 0; + virtual std::optional rampTime_mins() const = 0; + //============================================ "SETTER" MEMBER FUNCTIONS ============================================ - void setStepTime_mins (std::optional const val ); - void setStepTime_days (std::optional const val ); - void setStartTemp_c (std::optional const val ); + + // See model/StepBase.h for overrides of these + virtual void setStepTime_mins(std::optional const val) = 0; + virtual void setStepTime_days(std::optional const val) = 0; + virtual void setStartTemp_c (std::optional const val) = 0; + virtual void setRampTime_mins(std::optional const val) = 0; + void setEndTemp_c (std::optional const val ); void setStepNumber (int const stepNumber); void setOwnerId (int const ownerId ); // ⮜⮜⮜ All below added for BeerJSON support ⮞⮞⮞ void setDescription (QString const & val); - virtual void setRampTime_mins (std::optional const val); // See related Q_PROPERTY comment above for why this is virtual void setStartAcidity_pH(std::optional const val); void setEndAcidity_pH (std::optional const val); @@ -190,12 +199,6 @@ Q_OBJECT protected: virtual bool isEqualTo(NamedEntity const & other) const; - //! See comment above. By default stepTime_mins is optional - [[nodiscard]] virtual bool stepTimeIsRequired() const; - //! See comment above. By default startTemp_c is optional - [[nodiscard]] virtual bool startTempIsRequired() const; - //! See comment above. By default rampTime_mins is supported - [[nodiscard]] virtual bool rampTimeIsSupported() const; protected: std::optional m_stepTime_mins ; diff --git a/src/model/StepBase.h b/src/model/StepBase.h index 847bb6d8d..eac91e2f3 100755 --- a/src/model/StepBase.h +++ b/src/model/StepBase.h @@ -17,18 +17,95 @@ #define MODEL_STEPBASE_H #pragma once +#include + #include "database/ObjectStoreWrapper.h" #include "model/Recipe.h" +#include "model/Step.h" +#include "PhysicalConstants.h" #include "utils/CuriouslyRecurringTemplateBase.h" +#include "utils/TypeTraits.h" + +namespace { + // Where step time is required to have a value, this is the default + constexpr double defaultStepTime_mins {0.0}; + // Where start temperature is required to have a value, this is the default + constexpr double defaultStartTemp_c {22.2}; + + // Everything has to be double because our underlying measure (minutes) is allowed to be measured in fractions. + constexpr double minutesInADay = 24.0 * 60.0; + std::optional daysToMinutes(std::optional val) { + if (val) { + return *val * minutesInADay; + } + return std::nullopt; + } +} + +/** + * \brief Per the comments in model/Step.h on individual properties, there are a couple of cases where we want to be + * able to make minor changes between different subclasses -- eg \c stepTime_mins is required for \c MashStep but + * optional for \c BoilStep and \c FermentationStep. \c Step itself cannot be a templated class because the Qt + * Meta Object Compiler (moc) will barf out "Template classes not supported by Q_OBJECT", but we can still do + * most of the work at compile-time here. + */ +struct StepBaseOptions { + //! By default stepTime_mins is optional + bool stepTimeRequired = false; + //! By default startTemp_c is optional + bool startTempRequired = false; + //! By default rampTime_mins is not supported + bool rampTimeSupported = false; +}; +template struct has_stepTimeRequired : public std::integral_constant{}; +template struct has_startTempRequired: public std::integral_constant{}; +template struct has_rampTimeSupported: public std::integral_constant{}; +// See comment in utils/TypeTraits.h for definition of CONCEPT_FIX_UP (and why, for now, we need it) +template concept CONCEPT_FIX_UP StepTimeRequired = has_stepTimeRequired ::value; +template concept CONCEPT_FIX_UP StartTempRequired = has_startTempRequired::value; +template concept CONCEPT_FIX_UP RampTimeSupported = has_rampTimeSupported::value; /** * \brief Additional base class for \c MashStep, \c BoilStep, \c FermentationStep to provide strongly-typed functions * using CRTP. */ template class StepPhantom; -template +template class StepBase : public CuriouslyRecurringTemplateBase { protected: + StepBase() { + return; + } + + StepBase([[maybe_unused]] NamedParameterBundle const & namedParameterBundle) { + // We intend that Derived should always inherit from Step before it inherits from StepBase. So, at this point, + // the Step bits of Derived() will be constructed and initialised from namedParameterBundle. + + // + // If we're being constructed from a BeerXML file, we use the property stepTime_days for RECIPE > PRIMARY_AGE etc + // Otherwise we use the stepTime_mins property. + // + // See comment in Step.h for why we cannot do this in the Step constructor. + // + if (!SET_IF_PRESENT_FROM_NPB_NO_MV(StepBase::doSetStepTime_mins, namedParameterBundle, PropertyNames::Step::stepTime_mins) && + !SET_IF_PRESENT_FROM_NPB_NO_MV(StepBase::doSetStepTime_days, namedParameterBundle, PropertyNames::Step::stepTime_days)) { + this->derived().m_stepTime_mins = std::nullopt; + } + + + // Override the std::nullopt default for step time and/or start temp if subclass so requires + this->derived().m_stepTime_mins = this->correctStepTime_mins(this->derived().m_stepTime_mins); + this->derived().m_startTemp_c = this->correctStartTemp_c (this->derived().m_startTemp_c ); + + if (this->derived().m_rampTime_mins) { + this->checkRampTimeSupported(); + } + return; + } + + StepBase([[maybe_unused]] Derived const & other) { + return; + } /** * \brief Steps do not directly belong to a \c Recipe, but a step's owner (ie its \c Mash, \c Boil, \c Fermentation) @@ -42,6 +119,94 @@ class StepBase : public CuriouslyRecurringTemplateBase { return owner->getOwningRecipe(); } + //! No-op version + std::optional correctStepTime_mins(std::optional const val) const + requires (!StepTimeRequired) { + return val; + } + //! Substantive version + std::optional correctStepTime_mins(std::optional const val) const + requires (StepTimeRequired) { + // You might think we could assert here that either step time is optional or val is not std::nullopt. But it's + // not that simple, as generic code will have read that stepTime_mins is an optional field. So, the best we can + // do here is force std::nullopt to some default value; + return val.value_or(defaultStepTime_mins); + } + + std::optional doStepTime_mins() const { + // If subclass needs step time to be non-optional then we ensure std::nullopt cannot be returned + return this->correctStepTime_mins(this->derived().m_stepTime_mins); + } + void doSetStepTime_mins(std::optional const val) { + // Can't use SET_AND_NOTIFY macro here, but fortunately it's trivial + this->derived().setAndNotify(PropertyNames::Step::stepTime_mins, + this->derived().m_stepTime_mins, + this->correctStepTime_mins(val)); + return; + } + std::optional doStepTime_days() const { + auto const val = this->doStepTime_mins(); + // Convert minutes to days + if (val) { + return *val / minutesInADay; + } + return std::nullopt; + } + void doSetStepTime_days(std::optional const val) { + this->doSetStepTime_mins(daysToMinutes(val)); + return; + } + + //! No-op version + std::optional correctStartTemp_c(std::optional const val) const + requires (!StartTempRequired) { + return val; + } + //! Substantive version + std::optional correctStartTemp_c(std::optional const val) const + requires (StartTempRequired) { + return val.value_or(defaultStartTemp_c); + } + + std::optional doStartTemp_c () const { + return this->correctStartTemp_c(this->derived().m_startTemp_c); + } + void doSetStartTemp_c(std::optional const val) { + // Can't use SET_AND_NOTIFY macro here, but fortunately it's trivial + this->derived().setAndNotify(PropertyNames::Step::startTemp_c, + this->derived().m_startTemp_c, + this->derived().enforceMin(this->correctStartTemp_c(val), + "start temp", + PhysicalConstants::absoluteZero)); + return; + } + + //! No-op version + void checkRampTimeSupported() const requires (RampTimeSupported) { + return; + } + //! Substantive version + void checkRampTimeSupported() const requires (!RampTimeSupported) { + // It would probably be a coding error if we got here, but I _think_ it would also be harmless to carry on rather + // than assert. + qCritical().noquote() << + Q_FUNC_INFO << "Ramp time not supported for" << this->derived().metaObject()->className() << ":" << + Logging::getStackTrace(); + return; + } + + std::optional doRampTime_mins() const { + this->checkRampTimeSupported(); + return this->derived().m_rampTime_mins; + } + + void doSetRampTime_mins(std::optional const val) { + this->checkRampTimeSupported(); + // Can't use SET_AND_NOTIFY macro here, but fortunately it's trivial + this->derived().setAndNotify(PropertyNames::Step::rampTime_mins, this->derived().m_rampTime_mins, val); + return; + } + ObjectStore & doGetObjectStoreTypedInstance() const { return ObjectStoreTyped::getInstance(); } @@ -56,10 +221,11 @@ class StepBase : public CuriouslyRecurringTemplateBase { * * Note we have to be careful about comment formats in macro definitions */ -#define STEP_COMMON_DECL(NeName) \ +#define STEP_COMMON_DECL(NeName, Options) \ /* This allows StepBase to call protected and private members of Derived */ \ friend class StepBase; \ + NeName, \ + Options>; \ \ public: \ /* This alias makes it easier to template a number of functions */ \ @@ -68,15 +234,32 @@ class StepBase : public CuriouslyRecurringTemplateBase { \ virtual Recipe * getOwningRecipe() const; \ \ + virtual std::optional stepTime_mins() const override; \ + virtual std::optional stepTime_days() const override; \ + virtual std::optional startTemp_c () const override; \ + virtual std::optional rampTime_mins() const override; \ + virtual void setStepTime_mins(std::optional const val) override; \ + virtual void setStepTime_days(std::optional const val) override; \ + virtual void setStartTemp_c (std::optional const val) override; \ + virtual void setRampTime_mins(std::optional const val) override; \ + \ protected: \ /** Override NamedEntity::getObjectStoreTypedInstance */ \ - virtual ObjectStore & getObjectStoreTypedInstance() const; \ + virtual ObjectStore & getObjectStoreTypedInstance() const override; \ /** * \brief Derived classes should include this in their implementation file */ #define STEP_COMMON_CODE(NeName) \ - Recipe * NeName##Step::getOwningRecipe () const { return this->doGetOwningRecipe (); } \ + Recipe * NeName##Step::getOwningRecipe() const { return this->doGetOwningRecipe(); } \ + std::optional NeName##Step::stepTime_mins () const { return this->doStepTime_mins (); } \ + std::optional NeName##Step::stepTime_days () const { return this->doStepTime_days (); } \ + std::optional NeName##Step::startTemp_c () const { return this->doStartTemp_c (); } \ + std::optional NeName##Step::rampTime_mins () const { return this->doRampTime_mins (); } \ + void NeName##Step::setStepTime_mins(std::optional const val) { this->doSetStepTime_mins(val); return; } \ + void NeName##Step::setStepTime_days(std::optional const val) { this->doSetStepTime_days(val); return; } \ + void NeName##Step::setStartTemp_c (std::optional const val) { this->doSetStartTemp_c (val); return; } \ + void NeName##Step::setRampTime_mins(std::optional const val) { this->doSetRampTime_mins(val); return; } \ ObjectStore & NeName##Step::getObjectStoreTypedInstance() const { return this->doGetObjectStoreTypedInstance(); } \ #endif diff --git a/src/tableModels/BtTableModel.h b/src/tableModels/BtTableModel.h index ace16a4fd..406e2a097 100644 --- a/src/tableModels/BtTableModel.h +++ b/src/tableModels/BtTableModel.h @@ -40,7 +40,7 @@ class Recipe; template class TableModelBase; // This forward declaration is so we can make it a friend /** - * \class BtTableModelData + * \class BtTableModel * * \brief Typically used to show a grid list of \c Hop or \c Fermentable, etc in one of two contexts: * * Listing all items of this type in the database @@ -91,105 +91,6 @@ template class TableModelBase; // This forward declarat * because, by the magic of template metaprogramming \c TableModelBase "knows" whether its derived class inherits * from \c BtTableModelRecipeObserver or \c BtTableModelInventory, and can therefore adapt accordingly. */ -// TODO: Need to kill off BtTableModelData once we have switched everything over to using TableModelBase (just WaterTableModel left to do) -///template -///class BtTableModelData { -///protected: -/// BtTableModelData() : rows{} { -/// return; -/// } -/// // Need a virtual destructor as we have a virtual member function -/// virtual ~BtTableModelData() = default; -///public: -/// /** -/// * \brief Return the \c i-th row in the model. -/// * Returns \c nullptr on failure. -/// */ -/// std::shared_ptr getRow(int ii) const { -/// if (!(this->rows.isEmpty())) { -/// if (ii >= 0 && ii < this->rows.size()) { -/// return this->rows[ii]; -/// } -/// qWarning() << Q_FUNC_INFO << "index out of range (" << ii << "/" << this->rows.size() << ")"; -/// } else { -/// qWarning() << Q_FUNC_INFO << "this->rows is empty (" << ii << "/" << this->rows.size() << ")"; -/// } -/// return nullptr; -/// } -/// -/// /** -/// * \brief Remove duplicates and non-displayable items from the supplied list -/// */ -/// QList< std::shared_ptr > removeDuplicates(QList< std::shared_ptr > items, Recipe const * recipe = nullptr) { -/// decltype(items) tmp; -/// -/// for (auto ii : items) { -/// if (!recipe && (ii->deleted() || !ii->display())) { -/// continue; -/// } -/// if (!this->rows.contains(ii) ) { -/// tmp.append(ii); -/// } -/// } -/// return tmp; -/// } -/// -/// /** -/// * \brief Remove duplicates, ignoring if the item is displayed -/// */ -/// QList< std::shared_ptr > removeDuplicatesIgnoreDisplay(QList< std::shared_ptr > items, Recipe const * recipe = nullptr) { -/// decltype(items) tmp; -/// -/// for (auto ii : items) { -/// if (!recipe && ii->deleted() ) { -/// continue; -/// } -/// if (!this->rows.contains(ii) ) { -/// tmp.append(ii); -/// } -/// } -/// return tmp; -/// } -/// -/// /** -/// * \brief Given a raw pointer, find the index of the corresponding shared pointer in \c this->rows -/// * -/// * This is useful because the Qt signals and slots framework allows the slot receiving a signal to get a raw -/// * pointer to the object that sent the signal, and we often want to find the corresponding shared pointer in -/// * our list. -/// * -/// * Note that using this function is a lot safer than, say, calling ObjectStoreWrapper::getSharedFromRaw(), as -/// * that only works for objects that are already stored in the database, something which is not guaranteed to -/// * be the case with our rows. (Eg in SaltTableModel, new Salts are only stored in the DB when the window is -/// * closed with OK.) -/// * -/// * Function name is for consistency with \c QList::indexOf -/// * -/// * \param object what to search for -/// * \return index of object in this->rows or -1 if it's not found -/// */ -/// int findIndexOf(NE const * object) const { -/// for (int index = 0; index < this->rows.size(); ++index) { -/// if (this->rows.at(index).get() == object) { -/// return index; -/// } -/// } -/// return -1; -/// } -/// -///protected: -/// virtual std::shared_ptr getRowAsNamedEntity(int ii) { -/// return std::static_pointer_cast(this->getRow(ii)); -/// } -/// -/// QList< std::shared_ptr > rows; -///}; - -/*! - * \class BtTableModel - * - * \brief Shared interface & code for all the table models we use - */ class BtTableModel : public QAbstractTableModel { Q_OBJECT