From ee0125b0c953a835b9354b39233e109c084d26ec Mon Sep 17 00:00:00 2001 From: Matt Young Date: Tue, 5 Nov 2024 20:52:10 +0100 Subject: [PATCH 1/3] Fixes for adding and reordering steps --- src/database/DatabaseSchemaHelper.cpp | 53 ++++++ src/database/ObjectStore.cpp | 3 + src/model/Boil.h | 2 +- src/model/BoilStep.h | 2 +- src/model/BrewNote.h | 2 +- src/model/Equipment.h | 2 +- src/model/Fermentable.h | 2 +- src/model/Fermentation.h | 2 +- src/model/FermentationStep.h | 2 +- src/model/FolderBase.h | 2 +- src/model/Hop.h | 2 +- src/model/Ingredient.h | 2 +- src/model/IngredientAmount.h | 2 +- src/model/IngredientInRecipe.h | 2 +- src/model/Instruction.h | 2 +- src/model/Inventory.h | 2 +- src/model/Mash.h | 2 +- src/model/MashStep.h | 2 +- src/model/Misc.h | 2 +- src/model/NamedEntity.h | 2 +- src/model/OutlineableNamedEntity.h | 2 +- src/model/OwnedByRecipe.h | 2 +- src/model/OwnedSet.h | 221 +++++++++++++++------- src/model/Recipe.h | 2 +- src/model/RecipeAddition.h | 2 +- src/model/RecipeAdditionFermentable.h | 2 +- src/model/RecipeAdditionHop.h | 2 +- src/model/RecipeAdditionMisc.h | 2 +- src/model/RecipeAdditionYeast.h | 2 +- src/model/RecipeAdjustmentSalt.h | 2 +- src/model/RecipeUseOfWater.h | 2 +- src/model/Salt.h | 2 +- src/model/Step.h | 2 +- src/model/StepBase.h | 2 +- src/model/StepExtended.h | 2 +- src/model/StepOwnerBase.h | 255 +++++++++++++++++++++----- src/model/SteppedBase.h | 9 +- src/model/SteppedOwnerBase.h | 2 +- src/model/Style.h | 2 +- src/model/Water.h | 2 +- src/model/Yeast.h | 2 +- src/tableModels/StepTableModelBase.h | 40 ++-- 42 files changed, 481 insertions(+), 172 deletions(-) diff --git a/src/database/DatabaseSchemaHelper.cpp b/src/database/DatabaseSchemaHelper.cpp index ff2e5fe3..afb58c01 100644 --- a/src/database/DatabaseSchemaHelper.cpp +++ b/src/database/DatabaseSchemaHelper.cpp @@ -1073,6 +1073,59 @@ namespace { "WHERE reversed_step_number = 1 " "AND recipe.mash_id = last_mash_step.mash_id" ), {QVariant{false}, QVariant{true}}}, + // + // But wait, we're not done on pre-boil step. We also need to handle the case where a recipe has a mash that + // does not have any mash steps. Eg the supplied "Extract" recipes are like this. + // + // It may be there is a way to combine this with the SQL query above, but I think it's simpler not to and just + // live with some horrible copy-and-paste here in a query that just creates a (hopefully sane) pre-boil step + // for any recipes that don't have one. We make a heroic assumption that the start temperature is 15°C (ie + // about 60 °F) which is about what you might expect tap water temperature to be a lot of the time in a lot of + // places. + // + {QString("INSERT INTO boil_step (" + "name , " + "deleted , " + "display , " + "step_time_mins , " + "end_temp_c , " + "ramp_time_mins , " + "step_number , " + "boil_id , " + "description , " + "start_acidity_ph, " + "end_acidity_ph , " + "start_temp_c , " + "start_gravity_sg, " + "end_gravity_sg , " + "chilling_type " + ") SELECT " + "'Pre-boil for ' || recipe.name, " // name + "?, " // deleted + "?, " // display + "NULL, " // step_time_mins + "100.0, " // end_temp_c + "NULL, " // ramp_time_mins + "1, " // step_number + "recipe.boil_id, " // boil_id + "'Automatically-generated pre-boil step for ' || recipe.name, " // description + "NULL, " // start_acidity_ph + "NULL, " // end_acidity_ph + "15, " // start_temp_c + "NULL, " // start_gravity_sg + "NULL, " // end_gravity_sg + "NULL " // chilling_type + "FROM recipe " + "WHERE recipe.boil_id in ( " + "SELECT boil_id " + "FROM boil " + "WHERE boil_id NOT IN ( " + "SELECT boil_id " + "FROM boil_step " + "WHERE step_number = 1 " + ")" + ")" + ), {QVariant{false}, QVariant{true}}}, // Adding the second step for the actual boil itself is easier {QString("INSERT INTO boil_step (" "name , " diff --git a/src/database/ObjectStore.cpp b/src/database/ObjectStore.cpp index 2b56e3cb..0d18eda7 100644 --- a/src/database/ObjectStore.cpp +++ b/src/database/ObjectStore.cpp @@ -1061,6 +1061,9 @@ class ObjectStore::impl { qDebug() << Q_FUNC_INFO << "Updating" << object.metaObject()->className() << "property" << propertyName << "with database query" << queryString; + // Normally leave the next debug output commented, as it can generate a lot of logging. But it's useful to + // uncomment if you're seeing a lot of DB updates and the cause is not clear. +// qDebug().noquote() << Q_FUNC_INFO << Logging::getStackTrace(); // // Bind the values diff --git a/src/model/Boil.h b/src/model/Boil.h index 85e49ff3..db1623c8 100755 --- a/src/model/Boil.h +++ b/src/model/Boil.h @@ -31,7 +31,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Boil { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Boil { inline BtStringConst const property{#property}; } AddPropertyName(description ) AddPropertyName(notes ) AddPropertyName(preBoilSize_l) diff --git a/src/model/BoilStep.h b/src/model/BoilStep.h index 5423730a..ab9bd945 100755 --- a/src/model/BoilStep.h +++ b/src/model/BoilStep.h @@ -27,7 +27,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::BoilStep { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::BoilStep { inline BtStringConst const property{#property}; } AddPropertyName(chillingType) #undef AddPropertyName //=========================================== End of property name constants =========================================== diff --git a/src/model/BrewNote.h b/src/model/BrewNote.h index 871e8ed7..157ed7ac 100644 --- a/src/model/BrewNote.h +++ b/src/model/BrewNote.h @@ -34,7 +34,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::BrewNote { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::BrewNote { inline BtStringConst const property{#property}; } AddPropertyName(abv ) AddPropertyName(attenuation ) AddPropertyName(boilOff_l ) diff --git a/src/model/Equipment.h b/src/model/Equipment.h index 819efaf8..9222277e 100644 --- a/src/model/Equipment.h +++ b/src/model/Equipment.h @@ -33,7 +33,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Equipment { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Equipment { inline BtStringConst const property{#property}; } AddPropertyName(agingVesselLoss_l ) AddPropertyName(agingVesselNotes ) AddPropertyName(agingVesselType ) diff --git a/src/model/Fermentable.h b/src/model/Fermentable.h index b389ac30..481503fb 100644 --- a/src/model/Fermentable.h +++ b/src/model/Fermentable.h @@ -47,7 +47,7 @@ class RecipeAdditionFermentable; //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Fermentable { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Fermentable { inline BtStringConst const property{#property}; } AddPropertyName(alphaAmylase_dextUnits ) AddPropertyName(betaGlucan_ppm ) AddPropertyName(coarseFineDiff_pct ) diff --git a/src/model/Fermentation.h b/src/model/Fermentation.h index 3d54e6e5..316a91aa 100755 --- a/src/model/Fermentation.h +++ b/src/model/Fermentation.h @@ -33,7 +33,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Fermentation { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Fermentation { inline BtStringConst const property{#property}; } AddPropertyName(description ) ///AddPropertyName(fermentationSteps) AddPropertyName(notes ) diff --git a/src/model/FermentationStep.h b/src/model/FermentationStep.h index b23bf07c..9e189007 100755 --- a/src/model/FermentationStep.h +++ b/src/model/FermentationStep.h @@ -27,7 +27,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::FermentationStep { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::FermentationStep { inline BtStringConst const property{#property}; } AddPropertyName(freeRise) AddPropertyName(vessel ) #undef AddPropertyName diff --git a/src/model/FolderBase.h b/src/model/FolderBase.h index 88f09881..60770e16 100755 --- a/src/model/FolderBase.h +++ b/src/model/FolderBase.h @@ -26,7 +26,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::FolderBase { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::FolderBase { inline BtStringConst const property{#property}; } AddPropertyName(folder) #undef AddPropertyName //=========================================== End of property name constants =========================================== diff --git a/src/model/Hop.h b/src/model/Hop.h index 02989123..a44101ed 100644 --- a/src/model/Hop.h +++ b/src/model/Hop.h @@ -40,7 +40,7 @@ class RecipeAdditionHop; //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Hop { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Hop { inline BtStringConst const property{#property}; } AddPropertyName(alpha_pct ) AddPropertyName(beta_pct ) AddPropertyName(bPinene_pct ) diff --git a/src/model/Ingredient.h b/src/model/Ingredient.h index 568f0a4e..432513d6 100755 --- a/src/model/Ingredient.h +++ b/src/model/Ingredient.h @@ -27,7 +27,7 @@ class NamedParameterBundle; //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Ingredient { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Ingredient { inline BtStringConst const property{#property}; } AddPropertyName(totalInventory) #undef AddPropertyName //=========================================== End of property name constants =========================================== diff --git a/src/model/IngredientAmount.h b/src/model/IngredientAmount.h index 36f3cee2..f1e60c0b 100755 --- a/src/model/IngredientAmount.h +++ b/src/model/IngredientAmount.h @@ -26,7 +26,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::IngredientAmount { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::IngredientAmount { inline BtStringConst const property{#property}; } AddPropertyName(amount ) AddPropertyName(isWeight) // Deprecated. Used only for BeerXML support AddPropertyName(measure ) diff --git a/src/model/IngredientInRecipe.h b/src/model/IngredientInRecipe.h index 842c2d0e..d5c092ef 100755 --- a/src/model/IngredientInRecipe.h +++ b/src/model/IngredientInRecipe.h @@ -22,7 +22,7 @@ //╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::IngredientInRecipe { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::IngredientInRecipe { inline BtStringConst const property{#property}; } AddPropertyName(ingredientId ) #undef AddPropertyName //=========================================== End of property name constants =========================================== diff --git a/src/model/Instruction.h b/src/model/Instruction.h index cf8a06e8..124f3000 100644 --- a/src/model/Instruction.h +++ b/src/model/Instruction.h @@ -31,7 +31,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Instruction { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Instruction { inline BtStringConst const property{#property}; } AddPropertyName(completed ) AddPropertyName(directions) AddPropertyName(hasTimer ) diff --git a/src/model/Inventory.h b/src/model/Inventory.h index 5138fa50..365fa573 100644 --- a/src/model/Inventory.h +++ b/src/model/Inventory.h @@ -33,7 +33,7 @@ class TypeLookup; //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Inventory { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Inventory { inline BtStringConst const property{#property}; } AddPropertyName(ingredientId) #undef AddPropertyName //=========================================== End of property name constants =========================================== diff --git a/src/model/Mash.h b/src/model/Mash.h index dc2dcd40..e4929055 100644 --- a/src/model/Mash.h +++ b/src/model/Mash.h @@ -40,7 +40,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Mash { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Mash { inline BtStringConst const property{#property}; } AddPropertyName(equipAdjust ) AddPropertyName(grainTemp_c ) ///AddPropertyName(mashSteps ) diff --git a/src/model/MashStep.h b/src/model/MashStep.h index a0cf96f2..728586ed 100644 --- a/src/model/MashStep.h +++ b/src/model/MashStep.h @@ -33,7 +33,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::MashStep { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::MashStep { inline BtStringConst const property{#property}; } AddPropertyName(amount_l ) AddPropertyName(decoctionAmount_l ) // Should only be used for BeerXML AddPropertyName(infuseAmount_l ) // Should only be used for BeerXML diff --git a/src/model/Misc.h b/src/model/Misc.h index 9d972dd8..98f4495b 100644 --- a/src/model/Misc.h +++ b/src/model/Misc.h @@ -38,7 +38,7 @@ class RecipeAdditionMisc; //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Misc { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Misc { inline BtStringConst const property{#property}; } AddPropertyName(notes ) AddPropertyName(producer ) AddPropertyName(productId) diff --git a/src/model/NamedEntity.h b/src/model/NamedEntity.h index b3c4f29e..baef63f0 100644 --- a/src/model/NamedEntity.h +++ b/src/model/NamedEntity.h @@ -61,7 +61,7 @@ class Recipe; // IMPORTANT: These property names are unique within a class, but they are not globally unique, so we have to be a bit // careful about how we use them in look-ups. // -#define AddPropertyName(property) namespace PropertyNames::NamedEntity { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::NamedEntity { inline BtStringConst const property{#property}; } AddPropertyName(deleted) AddPropertyName(display) AddPropertyName(key) diff --git a/src/model/OutlineableNamedEntity.h b/src/model/OutlineableNamedEntity.h index b6bf15a6..117dc69d 100755 --- a/src/model/OutlineableNamedEntity.h +++ b/src/model/OutlineableNamedEntity.h @@ -22,7 +22,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::OutlineableNamedEntity { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::OutlineableNamedEntity { inline BtStringConst const property{#property}; } AddPropertyName(outline) #undef AddPropertyName //=========================================== End of property name constants =========================================== diff --git a/src/model/OwnedByRecipe.h b/src/model/OwnedByRecipe.h index c9b07864..7e29100a 100755 --- a/src/model/OwnedByRecipe.h +++ b/src/model/OwnedByRecipe.h @@ -24,7 +24,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::OwnedByRecipe { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::OwnedByRecipe { inline BtStringConst const property{#property}; } AddPropertyName(recipeId) #undef AddPropertyName //=========================================== End of property name constants =========================================== diff --git a/src/model/OwnedSet.h b/src/model/OwnedSet.h index 5a5a004b..82a374b6 100644 --- a/src/model/OwnedSet.h +++ b/src/model/OwnedSet.h @@ -30,10 +30,26 @@ struct OwnedSetOptions { /** * \brief In an enumerated set, items are in a strict order and are accordingly numbered (starting from 1) with a - * step number, which is accessed via \c seqNum / \c setSeqNum. \c OwnedSet::items returns items in - * this order. In a non-enumerated set, the \c OwnedSet class does not manage the order of the items and - * returns them in arbitrary order. (Typically there may be a weak ordering -- eg addition time for a - * \c RecipeAddition subclass or brew date for a \c BrewnNote -- but it is not managed inside of \c OwnedSet. + * step number, which is accessed via \c seqNum / \c setSeqNum. \c OwnedSet::items returns items in this + * order. + * + * In a non-enumerated set, the \c OwnedSet class does not manage the order of the items and returns them in + * arbitrary order. (Typically there may be a weak ordering -- eg addition time for a \c RecipeAddition + * subclass or brew date for a \c BrewnNote -- but it is not managed inside of \c OwnedSet. + * + * Note that \c setSeqNum has to take an optional second parmeter \c bool \c const \c notify (which should + * default to \c true). This is because, in an enumerated set, we need to be very careful and controlled + * about when we (either directly or indirectly) emit signals from our member functions. + * If we are part way through modifying a sequence of steps (eg to swap two steps or insert a new one) then + * we do not want other parts of the code to read the steps when they are in an "in between" state. Reading + * in such a state might either give incorrect data (eg two steps with the same step number) or, worse, make + * unwanted changes (eg normalising step numbers whilst we're in the middle of re-ordering steps). + * You might think that we can solve that with locks, but this is complicated because, usually, when we + * emit a signal, its slots are straight away run on the same thread before the signal function returns. This + * means the thread that you want to prevent from reading the step sequence is the same one that is already + * modifying it. So, instead, we ensure that whenever sequence numbers are being modified, no signal is + * emitted for the change of sequence number on each step. Instead, a single \c changed signal for the whole + * set is emitted at the end. */ bool enumerated = false; @@ -45,9 +61,6 @@ struct OwnedSetOptions { * on the newly-created copy is empty. This is, for instance, what we do with any \c BrewNote objects on a * \c Recipe. This is what \c copyable being \c false means. * - * - * - * * Note that it can never be possible to do a shallow copy of an \c OwnedSet, since an item in such a set * cannot meaningfully have two different owners. */ @@ -74,10 +87,11 @@ template concept CONCEPT_FIX_UP IsCopyable = is_Copyable * \param propertyName is the name of the property that we use to signal changes to the size of the owned set (eg item * added or removed). It is typically the name of the property holding this \c OwnedSet object, but the value * that we send with the \c changed signal is simply the new size of the set. - * \param itemChangedSlot is a member function slot on \c Owner that can receive \c NamedEntity::changed signals from - * \c Item objects in this set. Typically that member function just needs to call our \c acceptItemChange - * function. It is simpler to go via \c Owner because the owner object is able to call \c QObject::sender to - * get the sender and pass it to us. (From looking at the Qt source code eg at + * \param itemChangedSlot if not \c nullptr, is a member function slot on \c Owner that can receive + * \c NamedEntity::changed signals from \c Item objects in this set. (Otherwise, if it is \c nullptr, + * \c Owner::acceptStepChange will be used.) Typically that member function just needs to call our + * \c acceptItemChange function. It is simpler to go via \c Owner because the owner object is able to call + * \c QObject::sender to get the sender and pass it to us. (From looking at the Qt source code eg at * https://github.com/qt/qtbase/blob/dev/src/corelib/kernel/qobject.cpp, it seems \c QObject::sender will return * null if a slot on the object in question is not being invoked, so there is no use us trying to find ways to * call \c this->m_owner.sender(), as we'll always just get back \c nullptr.) @@ -90,6 +104,20 @@ template class OwnedSet { public: + + //! Non-virtual equivalent of isEqualTo + bool doIsEqualTo(OwnedSet const & other) const { + // + // Check each object has the same number of items and they're all the same + // + auto const myItems = this->items(); + auto const otherItems = other.items(); + return std::equal( myItems.begin(), myItems.end(), + otherItems.begin(), otherItems.end(), + [](std::shared_ptr const & lhs, + std::shared_ptr const & rhs) {return *lhs == *rhs;}); + } + /** * \brief Minimal constructor. Note that the reason we do not just initialise everything here is that the object * stores on which we depend might not yet themselves be initialised when this constructor is called. @@ -174,18 +202,6 @@ class OwnedSet { return; } -private: - /** - * \brief Connect an item's "changed" signal to us. - * - * Unusually, we don't worry about disconnecting this later. The \c Item object (\c item) will never belong - * to any other \c OwnedSet, and Qt will do the disconnection itself when \c item is destroyed. - */ - void connectItemChangedSignal(std::shared_ptr item) { - this->m_owner.connect(item.get(), &NamedEntity::changed, &this->m_owner, itemChangedSlot); - return; - } - /** * \brief Connect all our item's "changed" signals to us * @@ -198,34 +214,64 @@ class OwnedSet { return; } - //! No-op version - void putInOrder([[maybe_unused]] QList> items) const requires (!IsEnumerated) { +private: + /** + * \brief Connect an item's "changed" signal to us. + * + * Unusually, we don't worry about disconnecting this later. The \c Item object (\c item) will never belong + * to any other \c OwnedSet, and Qt will do the disconnection itself when \c item is destroyed. + */ + void connectItemChangedSignal(std::shared_ptr item) { + if constexpr (itemChangedSlot) { + this->m_owner.connect(item.get(), &NamedEntity::changed, &this->m_owner, itemChangedSlot); + } else { + this->m_owner.connect(item.get(), &NamedEntity::changed, &this->m_owner, &Owner::acceptStepChange); + } return; } - //! Substantive version - void putInOrder(QList> items) const requires (IsEnumerated) { + + void putInOrder(QList> & items) const requires (IsEnumerated) { std::sort(items.begin(), items.end(), [](std::shared_ptr const lhs, std::shared_ptr const rhs) { + // Per https://en.cppreference.com/w/cpp/algorithm/sort, this function needs to return "returns ​true + // if the first argument is less than (i.e. is ordered before) the second". return lhs->seqNum() < rhs->seqNum(); }); return; } - //! No-op version - void normaliseSeqNums([[maybe_unused]] QList> items) const requires (!IsEnumerated) { - return; - } - //! Substantive version - void normaliseSeqNums(QList> items) const requires (IsEnumerated) { - for (int ii = 0; ii < items.size(); ++ii) { - // - // Canonical item numbering starts from 1, which is +1 on canonical indexing! - // - int const canonicalSeqNum = ii + 1; - if (items[ii]->seqNum() != canonicalSeqNum) { - items[ii]->setSeqNum(canonicalSeqNum); + void normaliseSeqNums(QList> & items) const requires (IsEnumerated) { + int const ownerId = this->m_owner.key(); + for (int canonicalSeqNum = 1, prevSeqNum = 1; auto item : items) { + int const existingSeqNum = item->seqNum(); + // Normally leave this debug statement commented out as it generates too much logging, but can be useful for + // troubleshooting. +// qDebug() << +// Q_FUNC_INFO << Owner::staticMetaObject.className() << "#" << ownerId << ":" << +// Item::staticMetaObject.className() << "#" << item->key() << "sequence number is" << +// existingSeqNum << "; should be" << canonicalSeqNum; + + // It's a coding error if any item in the list does not belong to the owner + Q_ASSERT(item->ownerId() == ownerId); + + // Unfortunately, for historical reasons, we cannot assume that sequence (aka step) numbers read from the DB + // will always be unique for items belonging to a given owner (eg MashSteps in a Mash). So, although we can + // assert that step numbers never go down (because we ran them through sort), and that they are never less than + // 1, we cannot assert that they always go up! + Q_ASSERT(existingSeqNum >= prevSeqNum); + + // At this point, we correct things as best we can here so that the rest of the code can work on the basis of + // sequence numbers being in sequence starting from 1. + if (existingSeqNum != canonicalSeqNum) { + // We don't want a "changed" signal just because we normalised a sequence number. Firstly, the items are + // already in the correct order. We are just ensuring that all the sequence numbers are sequential and + // start from 1. Secondly, if we are calling this as part of a modification to the set, we only want one + // notification, at the end of whatever modification it is we are doing. + item->setSeqNum(canonicalSeqNum, false); } + prevSeqNum = existingSeqNum; + ++canonicalSeqNum; } return; } @@ -254,26 +300,40 @@ class OwnedSet { } // - // The object store does not guarantee what order it returned the items in, so, if they are enumerated, we need - // to put them in the right order. The same comment applies to our m_itemIds list. For enumerated sets, we - // _could_ enforce that the order in m_itemIds is the same as the ordering implied by seqNum() on the set - // members, but this would make other parts of the code a bit more complicated (where we share logic between - // enumerated and non-enumerated sets) for little if no gain. + // Couple of extra things we need to do for enumerated sets. Obviously the joy of templates is that we know at + // compile-time whether the set is enumerated or not, hence the constexpr here. // - this->putInOrder(items); + if constexpr (IsEnumerated) { + // + // The object store does not guarantee what order it returned the items in, so, if they are enumerated, we need + // to put them in the right order. The same comment applies to our m_itemIds list. For enumerated sets, we + // _could_ enforce that the order in m_itemIds is the same as the ordering implied by seqNum() on the set + // members, but this would make other parts of the code a bit more complicated (where we share logic between + // enumerated and non-enumerated sets) for little if no gain. + // + this->putInOrder(items); - // - // It can be that, although they are in the right order, the items are not canonically numbered. If this happens, - // it looks a bit odd in the UI -- eg because you have Instructions in a Recipe starting with Instruction #2 as - // the first one. We _could_ fix this in the UI layer, but it's easier to do it here -- and, since we're never - // talking about more than a handful of items (often less than 10, usually less than 20, pretty much always less - // than 30), the absolute overhead of doing so should be pretty small. - // - this->normaliseSeqNums(items); + // + // It can be that, although they are in the right order, the items are not canonically numbered. If this happens, + // it looks a bit odd in the UI -- eg because you have Instructions in a Recipe starting with Instruction #2 as + // the first one. We _could_ fix this in the UI layer, but it's easier to do it here -- and, since we're never + // talking about more than a handful of items (often less than 10, usually less than 20, pretty much always less + // than 30), the absolute overhead of doing so should be pretty small. + // + this->normaliseSeqNums(items); + } return items; } + /** + * \return Number of items in the set + */ + size_t size() const { + // We could optimise this a bit, but it's not noticeably hurting performance + return this->items().size(); + } + /** * \brief For the moment we only do the unenumerated version of this. For the enumerated version, we would implement * in terms of \c items(). @@ -290,6 +350,30 @@ class OwnedSet { } private: + /** + * \brief If we changed the set in any way, we call this function to have the owner emit a signal + * + * \param newSize If the caller already knows the set size, they pass it in to save us working it out. + */ + void emitSetChanged(std::optional const newSize = std::nullopt) { + auto const sizeToEmit {newSize.value_or(this->items().size())}; + qDebug() << + Q_FUNC_INFO << "Emitting set changed signal (size =" << sizeToEmit << ") for" << + Owner::staticMetaObject.className() << "#" << this->m_owner.key(); + emit this->m_owner.changed(this->m_owner.metaProperty(*propertyName), sizeToEmit); + + // + // For the moment at least, various things dealing with steps are expecting a specific signal stepsChanged rather + // than the generic NamedEntity::changed one, so send that too. (Non-step owners do not have the stepsChanged + // signal though.) + // + if constexpr (IsEnumerated) { + emit this->m_owner.stepsChanged(); + } + + return; + } + /** * \brief Adds a new item to the set. This is private because, for enumerated sets, we need to handle step number in * the public functions that calls this one -- so we don't want it to be possible for this to be called from @@ -332,7 +416,7 @@ class OwnedSet { } // Now we changed the size of the set, have the owner tell people about it - emit this->m_owner.changed(this->m_owner.metaProperty(*propertyName), this->items().size()); + this->emitSetChanged(); return item; } @@ -346,7 +430,7 @@ class OwnedSet { * \param seqNum counted from 1 (or 0 to append to the end of the list) */ std::shared_ptr insert(std::shared_ptr item, - int const seqNum) requires (IsEnumerated) { + int seqNum) requires (IsEnumerated) { auto existingItems = this->items(); // We'll treat any out of range sequence number as meaning "append to the end" @@ -357,10 +441,13 @@ class OwnedSet { // Note per https://en.cppreference.com/w/cpp/ranges/drop_view that dropping more than the number of elements is // OK (and just gives an empty range. for (auto existingItem : std::ranges::drop_view(existingItems, seqNum - 1)) { - existingItem->setSeqNum(existingItem->seqNum() + 1); + // Don't want to emit a "changed" signal here, as we're still part-way through modifying the sequence + existingItem->setSeqNum(existingItem->seqNum() + 1, false); } - item->setSeqNum(seqNum); + // Even here is a bit to early to emit a "changed" signal, as the item may need to be inserted in the DB. The + // extend member function call below will emit a "changed" signal for the whole set, which should be all we need. + item->setSeqNum(seqNum, false); return this->extend(item); } @@ -409,13 +496,13 @@ class OwnedSet { ObjectStoreWrapper::hardDelete(item); // - // Note that this call to items() will also call this->normaliseSeqNums(), so, in an enumerated set, item sequence + // Note that, in an enumerated set, this call to items() will also call this->normaliseSeqNums(), so item sequence // numbers will be adjusted/corrected for the fact that an item has been removed. // auto currentItems = this->items(); // Now we changed the size of the set, have the owner tell people about it - emit this->m_owner.changed(this->m_owner.metaProperty(*propertyName), currentItems.size()); + this->emitSetChanged(currentItems.size()); return item; } @@ -436,7 +523,7 @@ class OwnedSet { this->m_itemIds.clear(); // Now we changed the size of the set, have the owner tell people about it - emit this->m_owner.changed(this->m_owner.metaProperty(*propertyName), 0); + this->emitSetChanged(0); } return; } @@ -452,13 +539,6 @@ class OwnedSet { return; } - /** - * \return Number of items in the set - */ - size_t size() const { - return this->items().size(); - } - /*! * \brief Swap the positions of Items \c lhs and \c rhs in an enumerated set */ @@ -470,7 +550,7 @@ class OwnedSet { // It's also a coding error if we're trying to swap a item with itself Q_ASSERT(lhs.key() != rhs.key()); - this->normaliseSeqNums(); +/// this->normaliseSeqNums(); qDebug() << Q_FUNC_INFO << "Swapping items" << lhs.seqNum() << "(#" << lhs.key() << ") and " << rhs.seqNum() << " (#" << @@ -494,7 +574,7 @@ class OwnedSet { this->m_itemIds.swapItemsAt(lhsIndex, rhsIndex); } - emit this->m_owner.itemsChanged(); + this->emitSetChanged(); return; } @@ -603,5 +683,4 @@ class OwnedSet { QVector m_itemIds = {}; }; - #endif diff --git a/src/model/Recipe.h b/src/model/Recipe.h index 3af3c939..16db2d66 100644 --- a/src/model/Recipe.h +++ b/src/model/Recipe.h @@ -47,7 +47,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Recipe { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Recipe { inline BtStringConst const property{#property}; } AddPropertyName(ABV_pct ) AddPropertyName(age_days ) AddPropertyName(ageTemp_c ) diff --git a/src/model/RecipeAddition.h b/src/model/RecipeAddition.h index 1bc80941..54c9cb0e 100755 --- a/src/model/RecipeAddition.h +++ b/src/model/RecipeAddition.h @@ -25,7 +25,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::RecipeAddition { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::RecipeAddition { inline BtStringConst const property{#property}; } AddPropertyName(stage ) AddPropertyName(step ) AddPropertyName(addAtTime_mins ) diff --git a/src/model/RecipeAdditionFermentable.h b/src/model/RecipeAdditionFermentable.h index d16e700d..4bcae0d2 100755 --- a/src/model/RecipeAdditionFermentable.h +++ b/src/model/RecipeAdditionFermentable.h @@ -28,7 +28,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::RecipeAdditionFermentable { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::RecipeAdditionFermentable { inline BtStringConst const property{#property}; } AddPropertyName(fermentable) AddPropertyName(use) // Deprecated - retained only for BeerXML diff --git a/src/model/RecipeAdditionHop.h b/src/model/RecipeAdditionHop.h index 65196009..63204fbf 100755 --- a/src/model/RecipeAdditionHop.h +++ b/src/model/RecipeAdditionHop.h @@ -28,7 +28,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::RecipeAdditionHop { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::RecipeAdditionHop { inline BtStringConst const property{#property}; } AddPropertyName(hop) AddPropertyName(use) // Deprecated - retained only for BeerXML diff --git a/src/model/RecipeAdditionMisc.h b/src/model/RecipeAdditionMisc.h index 7205e652..d6b9e9c3 100755 --- a/src/model/RecipeAdditionMisc.h +++ b/src/model/RecipeAdditionMisc.h @@ -28,7 +28,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::RecipeAdditionMisc { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::RecipeAdditionMisc { inline BtStringConst const property{#property}; } AddPropertyName(misc) AddPropertyName(use) // Deprecated - retained only for BeerXML diff --git a/src/model/RecipeAdditionYeast.h b/src/model/RecipeAdditionYeast.h index d5486e28..15341a05 100755 --- a/src/model/RecipeAdditionYeast.h +++ b/src/model/RecipeAdditionYeast.h @@ -28,7 +28,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::RecipeAdditionYeast { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::RecipeAdditionYeast { inline BtStringConst const property{#property}; } AddPropertyName(addToSecondary ) // Deprecated - retained only for BeerXML AddPropertyName(attenuation_pct ) AddPropertyName(yeast ) diff --git a/src/model/RecipeAdjustmentSalt.h b/src/model/RecipeAdjustmentSalt.h index df3ebfa5..1a7974c9 100755 --- a/src/model/RecipeAdjustmentSalt.h +++ b/src/model/RecipeAdjustmentSalt.h @@ -28,7 +28,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::RecipeAdjustmentSalt { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::RecipeAdjustmentSalt { inline BtStringConst const property{#property}; } AddPropertyName(salt ) AddPropertyName(whenToAdd) #undef AddPropertyName diff --git a/src/model/RecipeUseOfWater.h b/src/model/RecipeUseOfWater.h index 6f478d48..94627e92 100755 --- a/src/model/RecipeUseOfWater.h +++ b/src/model/RecipeUseOfWater.h @@ -26,7 +26,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::RecipeUseOfWater { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::RecipeUseOfWater { inline BtStringConst const property{#property}; } AddPropertyName(recipeId ) AddPropertyName(water ) AddPropertyName(volume_l ) diff --git a/src/model/Salt.h b/src/model/Salt.h index 180931ea..c91cd848 100644 --- a/src/model/Salt.h +++ b/src/model/Salt.h @@ -36,7 +36,7 @@ class RecipeAdjustmentSalt; //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Salt { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Salt { inline BtStringConst const property{#property}; } AddPropertyName(isAcid ) AddPropertyName(percentAcid ) AddPropertyName(type ) diff --git a/src/model/Step.h b/src/model/Step.h index 0c34feef..5ddc643a 100755 --- a/src/model/Step.h +++ b/src/model/Step.h @@ -28,7 +28,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Step { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Step { inline BtStringConst const property{#property}; } AddPropertyName(description ) AddPropertyName(endAcidity_pH ) AddPropertyName(endTemp_c ) diff --git a/src/model/StepBase.h b/src/model/StepBase.h index 31d46077..6bfd655d 100755 --- a/src/model/StepBase.h +++ b/src/model/StepBase.h @@ -29,7 +29,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::StepBase { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::StepBase { inline BtStringConst const property{#property}; } AddPropertyName(rampTime_mins) AddPropertyName(startTemp_c ) AddPropertyName(stepTime_days) // Mostly needed for BeerXML diff --git a/src/model/StepExtended.h b/src/model/StepExtended.h index a5a388a8..c55ecc00 100755 --- a/src/model/StepExtended.h +++ b/src/model/StepExtended.h @@ -22,7 +22,7 @@ //╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::StepExtended { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::StepExtended { inline BtStringConst const property{#property}; } AddPropertyName(startGravity_sg) AddPropertyName( endGravity_sg) #undef AddPropertyName diff --git a/src/model/StepOwnerBase.h b/src/model/StepOwnerBase.h index eb9ee365..bd351e81 100755 --- a/src/model/StepOwnerBase.h +++ b/src/model/StepOwnerBase.h @@ -27,8 +27,21 @@ #include "database/ObjectStoreWrapper.h" #include "model/Recipe.h" -#include "model/SteppedOwnerBase.h" +///#include "model/SteppedOwnerBase.h" +#include "utils/CuriouslyRecurringTemplateBase.h" +#include "model/OwnedSet.h" +//====================================================================================================================== +//========================================== Start of property name constants ========================================== +// See comment in model/NamedEntity.h +#define AddPropertyName(property) namespace PropertyNames::StepOwnerBase { inline BtStringConst const property{#property}; } +AddPropertyName(numSteps) +AddPropertyName(steps ) +#undef AddPropertyName +//=========================================== End of property name constants =========================================== +//====================================================================================================================== + +#define StepOwnerBaseOptions OwnedSetOptions{ .enumerated = true } /** * \brief Templated base class for \c Mash, \c Boil and \c Fermentation to handle manipulation of their component steps * (\c MashStep, \c BoilStep and \c FermentationStep respectively). @@ -64,56 +77,193 @@ * * We assume/require that \c DerivedStep inherits from \c Step. * - * Note that we do \b not inherit from \c CuriouslyRecurringTemplateBase because \c SteppedOwnerBase already does - * this. If we inherited again, we'd end up with two (identical) implementations of this->derived() that the - * compiler can't disambiguate between. + * TBD: For the moment, we have \c OwnedSet as a member, with wrapper functions. We could, instead, inherit from + * \c OwnedSet, and use its interface directly, at the cost of a bit of renaming. */ template class StepOwnerPhantom; template -class StepOwnerBase : public SteppedOwnerBase { -public: +class StepOwnerBase : public CuriouslyRecurringTemplateBase { // Note that, because this is static, it cannot be initialised inside the class definition static TypeLookup const typeLookup; + // This allows Derived to call our protected and private members + friend Derived; + +private: + //! Non-virtual equivalent of isEqualTo + bool doIsEqualTo(StepOwnerBase const & other) const { + return this->m_stepSet.doIsEqualTo(other.m_stepSet); + } + StepOwnerBase() : - SteppedOwnerBase{} { + m_stepSet{this->derived()} { return; } - StepOwnerBase(NamedParameterBundle const & namedParameterBundle) : - SteppedOwnerBase{namedParameterBundle} { + StepOwnerBase([[maybe_unused]] NamedParameterBundle const & namedParameterBundle) : + // See comment in OwnedSet for why it never needs the NamedParameterBundle + m_stepSet{this->derived()} { return; } - StepOwnerBase(Derived const & other) : - SteppedOwnerBase{other} { + StepOwnerBase(StepOwnerBase const & other) : + m_stepSet{this->derived(), other.m_stepSet} { return; } /** - * \brief We have to delete the default copy constructor because we want the constructor above (that takes \c Derived - * rather than \c SteppedOwnerBase) to be used instead of a compiler-generated copy constructor which wouldn't - * do the deep copy we need. + * \brief We don't want copy assignment happening. */ - StepOwnerBase(StepOwnerBase const & other) = delete; + StepOwnerBase & operator=(StepOwnerBase const & other) = delete; + + ~StepOwnerBase() = default; + + // TBD: This public block of member functions is just a wrapper around the OwnedSet interface. We could get rid of + // it if we inherited from OwnedSet. +public: + QList> steps() const { + return this->m_stepSet.items(); + } /** - * \brief Similarly, we don't want copy assignment happening. + * \brief Returns the step at the specified position, if it exists, or \c nullptr if not + * + * \param seqNum counted from 1 */ - StepOwnerBase & operator=(StepOwnerBase const & other) = delete; + std::shared_ptr stepAt(int const seqNum) const { + return this->m_stepSet.itemAt(seqNum); + } - ~StepOwnerBase() = default; + /** + * \brief Inserts a new step at the specified position. If there is already a step in that position, it (and all + * subsequent ones) will be bumped one place down the list. + * + * \param step + * \param seqNum counted from 1 + */ + std::shared_ptr insertStep(std::shared_ptr step, int const seqNum) { + return this->m_stepSet.insert(step, seqNum); + } + + /** + * \brief Adds a new step at the end of the current list + */ + std::shared_ptr addStep(std::shared_ptr step) { + return this->m_stepSet.add(step); + } + + std::shared_ptr removeStep(std::shared_ptr step) { + return this->m_stepSet.remove(step); + } + + /** + * \brief Sets (or unsets) the step at the specified position. + * + * Note this is different from insertStep(), as: + * - If there is a step in the specified position it will be overwritten rather than bumped down the list + * - Calling this with non-null value (ie not std::nullopt) for second and later steps will ensure prior + * step(s) exist by creating default ones if necessary. + * - Calling this with null value (ie std::nullopt) delete any subsequent steps. (Doesn't make sense for + * third step to become second in the context of this function.) + * + * \param step The step to set, or \c nullptr to unset it + * \param seqNum + */ + void setStepAt(std::shared_ptr step, int const seqNum) { + this->m_stepSet.setAt(step, seqNum); + return; + } + void setSteps(QList> const & steps) { + this->m_stepSet.setAll(steps); + return; + } + + unsigned int numSteps() const { + return this->m_stepSet.size(); + } + + /*! + * \brief Swap Steps \c step1 and \c step2 + */ + void swapSteps(DerivedStep & lhs, DerivedStep & rhs) { + this->m_stepSet.swap(lhs, rhs); + return; + } + + void removeAllSteps() { + this->m_stepSet.removeAll(); + return; + } + +private: + void doSetKey(int key) { + // First call the base class function + this->derived().NamedEntity::setKey(key); + // Now tell the OwnedSet about the new key + this->m_stepSet.doSetKey(key); + return; + } + + /** + * \brief Connect DerivedStep changed signals to their parent Derived objects. + * + * Needs to be called \b after all the calls to ObjectStoreTyped::getInstance().loadAll() + */ + static void doConnectSignals() { + for (auto dd : ObjectStoreTyped::getInstance().getAllRaw()) { + dd->m_stepSet.connectAllItemChangedSignals(); + } + return; + } + + /** + * \brief Intended to be called from \c Derived::acceptStepChange + * + * \param sender - Result of caller calling \c this->sender() (which is protected, so we can't call it here) + * \param prop - As received by Derived::acceptStepChange + * \param val - As received by Derived::acceptStepChange + * \param additionalProperties - Additional properties for which to emit \c changed signal if the change we are + * receiving comes from one of our steps. TODO: Remove this + */ + void doAcceptStepChange(QObject * sender, + QMetaProperty prop, + QVariant val, + [[maybe_unused]] QList const additionalProperties = {}) { + DerivedStep * stepSender = qobject_cast(sender); + if (!stepSender) { + return; + } + + this->m_stepSet.acceptItemChange(*stepSender, prop, val); + return; + } + + //================================================ Member variables ================================================= + OwnedSet m_stepSet; }; template TypeLookup const StepOwnerBase::typeLookup { "StepOwnerBase", { - // We don't have any properties + // + // See comment in model/IngredientAmount.h for why we can't use the PROPERTY_TYPE_LOOKUP_ENTRY or + // PROPERTY_TYPE_LOOKUP_ENTRY_NO_MV macros here. + // + {&PropertyNames::StepOwnerBase::numSteps, + TypeInfo::construct>( + PropertyNames::StepOwnerBase::numSteps, + TypeLookupOf>::value, + NonPhysicalQuantity::OrdinalNumeral + )}, }, - // Parent class lookup - {&SteppedOwnerBase::typeLookup} + // Parent class lookup: none as we are at the top of this arm of the inheritance tree + {} }; /** @@ -125,37 +275,46 @@ TypeLookup const StepOwnerBase::typeLookup { * Note we have to be careful about comment formats in macro definitions */ #define STEP_OWNER_COMMON_DECL(NeName, LcNeName) \ - STEPPED_OWNER_COMMON_DECL(NeName, NeName##Step) \ - /* This allows StepOwnerBase to call protected and private members of Derived */ \ - friend class StepOwnerBase; \ - \ - public: \ - \ - /** \brief Overrides \c NamedEntity::setKey() */ \ - virtual void setKey(int key) override; \ - \ - /** \brief Overrides \c NamedEntity::hardDeleteOwnedEntities() */ \ - /* Derived owns its DerivedSteps so needs to delete them if it */ \ - /* itself is being deleted */ \ - virtual void hardDeleteOwnedEntities() override; \ - \ - /* TODO These are aliases we should probably get rid of */ \ - QList> LcNeName##Steps() const; \ - void set##NeName##Steps(QList> const & val); \ + /* This allows StepOwnerBase to call protected and private members of Derived */ \ + friend class StepOwnerBase; \ + \ + public: \ + /* This alias makes it easier to template a number of functions that are */ \ + /* essentially the same for all "Step Owner" classes. */ \ + using StepClass = NeName##Step; \ + \ + /* Relational getters and setters */ \ + QList> LcNeName##Steps () const; \ + void set##NeName##Steps (QList> const & val); \ + \ + /** \brief Connect DerivedStep changed signals to their parent Mashes. */ \ + /* Needs to be called \b after all the calls to */ \ + /* ObjectStoreTyped::getInstance().loadAll() */ \ + static void connectSignals(); \ + \ + virtual void setKey(int key) override; \ + \ + /** \brief NeName owns its NeName##Steps so needs to delete them if it */ \ + /* itself is being deleted */ \ + virtual void hardDeleteOwnedEntities() override; \ + /** * \brief Derived classes should include this in their implementation file */ #define STEP_OWNER_COMMON_CODE(NeName, LcNeName) \ - STEPPED_OWNER_COMMON_CODE(NeName, NeName##Step) \ - void NeName::setKey(int key) { this->doSetKey(key); return; } \ - \ - void NeName::hardDeleteOwnedEntities() { this->doHardDeleteOwnedEntities(); return; } \ - \ - QList> NeName::LcNeName##Steps() const { return this->steps(); } \ - void NeName::set##NeName##Steps(QList> const & val) { \ - this->setSteps(val); return; \ - } \ + QList> NeName::LcNeName##Steps() const { \ + return this->m_stepSet.items(); \ + } \ + void NeName::set##NeName##Steps(QList> const & val) { \ + this->m_stepSet.setAll(val); return; \ + } \ + \ + void NeName::connectSignals() { StepOwnerBase::doConnectSignals(); return; } \ + \ + void NeName::setKey(int key) { this->doSetKey(key); return; } \ + \ + void NeName::hardDeleteOwnedEntities() { this->m_stepSet.doHardDeleteOwnedEntities(); return; } \ #endif diff --git a/src/model/SteppedBase.h b/src/model/SteppedBase.h index 9aec3233..237c062b 100644 --- a/src/model/SteppedBase.h +++ b/src/model/SteppedBase.h @@ -25,7 +25,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::SteppedBase { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::SteppedBase { inline BtStringConst const property{#property}; } AddPropertyName(ownerId ) AddPropertyName(stepNumber) #undef AddPropertyName @@ -106,7 +106,9 @@ class SteppedBase : public CuriouslyRecurringTemplateBasem_ownerId ; } + // TODO: Merge these two functions int stepNumber() const { return this->m_stepNumber; } + int seqNum () const { return this->m_stepNumber; } void setOwnerId (int const val) { this->m_ownerId = val; @@ -129,6 +131,11 @@ class SteppedBase : public CuriouslyRecurringTemplateBasesetStepNumber(val, notify); + return; + } /** * \brief This is, amongst other things, needed by \c TreeModelBase diff --git a/src/model/SteppedOwnerBase.h b/src/model/SteppedOwnerBase.h index ef980b33..12d39089 100644 --- a/src/model/SteppedOwnerBase.h +++ b/src/model/SteppedOwnerBase.h @@ -27,7 +27,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::SteppedOwnerBase { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::SteppedOwnerBase { inline BtStringConst const property{#property}; } AddPropertyName(numSteps) AddPropertyName(steps ) #undef AddPropertyName diff --git a/src/model/Style.h b/src/model/Style.h index a4c7b15e..7feac9fb 100644 --- a/src/model/Style.h +++ b/src/model/Style.h @@ -33,7 +33,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Style { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Style { inline BtStringConst const property{#property}; } AddPropertyName(abvMax_pct ) AddPropertyName(abvMin_pct ) AddPropertyName(appearance ) diff --git a/src/model/Water.h b/src/model/Water.h index ffc297b3..9bb6f8f6 100644 --- a/src/model/Water.h +++ b/src/model/Water.h @@ -32,7 +32,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Water { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Water { inline BtStringConst const property{#property}; } AddPropertyName(alkalinity_ppm ) AddPropertyName(alkalinityAsHCO3) ///AddPropertyName(amount ) diff --git a/src/model/Yeast.h b/src/model/Yeast.h index 0e2ee5ae..c3fd952b 100644 --- a/src/model/Yeast.h +++ b/src/model/Yeast.h @@ -38,7 +38,7 @@ class RecipeAdditionYeast; //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::Yeast { BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::Yeast { inline BtStringConst const property{#property}; } AddPropertyName(alcoholTolerance_pct ) AddPropertyName(attenuationTypical_pct ) // This is for use only by BeerXML AddPropertyName(attenuationMax_pct ) diff --git a/src/tableModels/StepTableModelBase.h b/src/tableModels/StepTableModelBase.h index 110e92ef..b6920521 100755 --- a/src/tableModels/StepTableModelBase.h +++ b/src/tableModels/StepTableModelBase.h @@ -17,6 +17,8 @@ #define TABLEMODELS_STEPTABLEMODELBASE_H #pragma once +#include + #include #include "utils/CuriouslyRecurringTemplateBase.h" @@ -64,6 +66,9 @@ class StepTableModelBase : public CuriouslyRecurringTemplateBasem_stepOwnerObs != stepOwner) { + qDebug() << + Q_FUNC_INFO << "Connecting stepsChanged signal from" << StepOwnerClass::staticMetaObject.className() << "#" << + stepOwner->key(); this->derived().connect(stepOwner.get(), &StepOwnerClass::stepsChanged, &this->derived(), &Derived::stepOwnerChanged); } @@ -118,6 +123,7 @@ class StepTableModelBase : public CuriouslyRecurringTemplateBase step, int current) { // doSomething will be -1 if we are moving up and 1 if we are moving down // and 0 if nothing is to be done (see next comment) @@ -169,6 +175,7 @@ class StepTableModelBase : public CuriouslyRecurringTemplateBasem_stepOwnerObs || stepNum == 0 || stepNum >= this->derived().rows.size()) { return; @@ -188,14 +195,14 @@ class StepTableModelBase : public CuriouslyRecurringTemplateBasem_stepOwnerObs; this->setStepOwner(this->m_stepOwnerObs); return; } void doStepChanged(QMetaProperty prop, [[maybe_unused]] QVariant val) { - qDebug() << Q_FUNC_INFO; + qDebug() << Q_FUNC_INFO << prop.name(); StepClass * stepSender = qobject_cast(this->derived().sender()); if (stepSender) { @@ -213,6 +220,7 @@ class StepTableModelBase : public CuriouslyRecurringTemplateBasederived().findIndexOf(stepSender); if (ii >= 0) { if (prop.name() == PropertyNames::SteppedBase::stepNumber) { + qDebug().noquote() << Q_FUNC_INFO << Logging::getStackTrace(); this->reorderStep(this->derived().rows.at(ii), ii); } @@ -240,20 +248,20 @@ class StepTableModelBase : public CuriouslyRecurringTemplateBase; \ - \ - public: \ - void set##StepOwnerName(std::shared_ptr stepOwner); \ - std::shared_ptr get##StepOwnerName() const; \ - bool removeStep(std::shared_ptr step); \ - \ - public slots: \ - void moveStepUp(int stepNum); \ - void moveStepDown(int stepNum); \ - void stepOwnerChanged(); \ - void stepChanged(QMetaProperty prop, QVariant val); \ +#define STEP_TABLE_MODEL_COMMON_DECL(StepOwnerName) \ + /* This allows StepTableModelBase to call protected and private members of Derived */ \ + friend StepTableModelBase; \ + \ + public: \ + void set##StepOwnerName(std::shared_ptr stepOwner); \ + std::shared_ptr get##StepOwnerName() const; \ + bool removeStep(std::shared_ptr step); \ + \ + public slots: \ + void moveStepUp(int stepNum); \ + void moveStepDown(int stepNum); \ + void stepOwnerChanged(); \ + void stepChanged(QMetaProperty prop, QVariant val); \ /** From e399e737a2f563d23f75d6183e6c7c8747dcc886 Mon Sep 17 00:00:00 2001 From: Matt Young Date: Wed, 6 Nov 2024 17:24:01 +0100 Subject: [PATCH 2/3] More tidy up, plus add translators to about menu --- CHANGES.markdown | 12 + src/AboutDialog.cpp | 20 +- src/BrewDayFormatter.cpp | 4 +- src/BrewDayScrollWidget.cpp | 20 +- src/MainWindow.cpp | 2 +- src/RecipeFormatter.cpp | 4 +- src/WaterDialog.h | 2 +- src/database/ObjectStoreTyped.cpp | 16 +- src/model/Boil.h | 2 +- src/model/BoilStep.h | 2 +- src/model/{SteppedBase.h => EnumeratedBase.h} | 98 ++-- src/model/Fermentation.h | 2 +- src/model/FermentationStep.h | 2 +- src/model/Instruction.cpp | 14 +- src/model/Instruction.h | 10 +- src/model/Mash.h | 2 +- src/model/MashStep.h | 2 +- src/model/Recipe.cpp | 235 ++++---- src/model/Recipe.h | 108 ++-- src/model/StepBase.h | 22 +- src/model/StepOwnerBase.h | 1 - src/model/SteppedOwnerBase.h | 544 ------------------ src/serialization/json/BeerJson.cpp | 12 +- src/serialization/xml/BeerXml.cpp | 6 +- src/serialization/xml/XmlMashRecord.cpp | 2 +- src/serialization/xml/XmlRecipeRecord.cpp | 2 +- src/tableModels/StepTableModelBase.h | 4 +- src/trees/TreeFilterProxyModel.cpp | 1 + 28 files changed, 297 insertions(+), 854 deletions(-) rename src/model/{SteppedBase.h => EnumeratedBase.h} (71%) delete mode 100644 src/model/SteppedOwnerBase.h diff --git a/CHANGES.markdown b/CHANGES.markdown index 2379c2c7..49290203 100644 --- a/CHANGES.markdown +++ b/CHANGES.markdown @@ -13,6 +13,18 @@ happens, so I'm now setting it to a slightly arbitrary time early in the morning * Additional methods for calculating IBU * We'll list other new features here... +## v4.0.10 +Bug fixes and minor enhancements. + +### New Features +* Danish translation + +### Bug Fixes +* Crash when copying recipe, then on adding new steps in mash, ferment, boil, others [868](https://github.com/Brewtarget/brewtarget/issues/868) + +### Release Timestamp +Sun, 3 Nov 2024 04:00:10 +0100 + ## v4.0.9 Bug fixes and minor enhancements. diff --git a/src/AboutDialog.cpp b/src/AboutDialog.cpp index f5b5b07e..bfde683c 100755 --- a/src/AboutDialog.cpp +++ b/src/AboutDialog.cpp @@ -164,14 +164,20 @@ AboutDialog::AboutDialog(QWidget * parent) : // 2013 U-CHIMCHIM\mik // Incomplete name and email " " "" - // ********************************************************************************************************** - // * Note that the HTML source indentation here is different than above so that we don't pick up testers as * - // * copyright holders in the awk command above! * - // ********************************************************************************************************** - "

The following people have made notable contributions with testing and bug reports:

" + // *********************************************************************************************************** + // * Note that the HTML source indentation here is different than above so that we don't pick up translators * + // * as software copyright holders in the awk command above! * + // * * + // * This list is currently somewhat incomplete, partly as I haven't yet found a record of who did all the * + // * original translations, and partly as some GitHub users do not have their name in their profile. * + // *********************************************************************************************************** + "

The following people are amongst those who have provided translations:

" "
    " - "
  • Mik Firestone <mikfire@gmail.com>
    • " - "
  • Nikolas "Jazzbeerman"
    • " + "
  • André Rodrigues (Brazilian Portuguese)
    • " + "
  • Orla Valbjørn Møller (Danish)
    • " + "
  • Marcel Koek (Dutch)
    • " + "
  • Mattias Måhl (Swedish)
    • " + "
  • Mikhail Gorbunov (Russian)
    • " "
" "" "

License (GPLv3)

" diff --git a/src/BrewDayFormatter.cpp b/src/BrewDayFormatter.cpp index cc5ea335..e2b4b529 100644 --- a/src/BrewDayFormatter.cpp +++ b/src/BrewDayFormatter.cpp @@ -197,7 +197,7 @@ QString BrewDayFormatter::buildInstructionHtml() { .arg(tr("Step")); bool useAlt = true; - for (auto instruction : recObs->steps()) { + for (auto instruction : recObs->instructions()) { useAlt = !useAlt; QString stepTime, tmp; QList reagents; @@ -259,7 +259,7 @@ QList BrewDayFormatter::buildInstructionList() { ret.append(row); row.clear(); - for (auto instruction : recObs->steps()) { + for (auto instruction : recObs->instructions()) { QString stepTime, tmp; QList reagents; diff --git a/src/BrewDayScrollWidget.cpp b/src/BrewDayScrollWidget.cpp index e8ce545d..20067348 100644 --- a/src/BrewDayScrollWidget.cpp +++ b/src/BrewDayScrollWidget.cpp @@ -79,7 +79,7 @@ BrewDayScrollWidget::BrewDayScrollWidget(QWidget* parent) : QWidget{parent}, BrewDayScrollWidget::~BrewDayScrollWidget() = default; void BrewDayScrollWidget::saveInstruction() { - this->m_recObs->steps()[ listWidget->currentRow() ]->setDirections( btTextEdit->toPlainText() ); + this->m_recObs->instructions()[ listWidget->currentRow() ]->setDirections( btTextEdit->toPlainText() ); return; } @@ -114,7 +114,7 @@ void BrewDayScrollWidget::generateInstructions() { // TODO: Would be neat to make this an undoable action // bool proceed = true; - if (this->m_recObs->numSteps() > 0) { + if (this->m_recObs->instructions().size() > 0) { proceed = QMessageBox::Yes == QMessageBox::question( this, tr("Overwrite Existing Instructions"), @@ -143,7 +143,7 @@ void BrewDayScrollWidget::removeSelectedInstruction() { if (row < 0) { return; } - this->m_recObs->removeStep(this->m_recIns[row]); + this->m_recObs->m_instructions.remove(this->m_recIns[row]); // After updating the model, this is the simplest way to update the display this->setRecipe(this->m_recObs); @@ -171,7 +171,7 @@ void BrewDayScrollWidget::pushInstructionUp() { return; } - this->m_recObs->swapSteps(*this->m_recIns[row], *this->m_recIns[row-1]); + this->m_recObs->m_instructions.swap(*this->m_recIns[row], *this->m_recIns[row-1]); // After updating the model, this is the simplest way to update the display this->setRecipe(this->m_recObs); @@ -190,7 +190,7 @@ void BrewDayScrollWidget::pushInstructionDown() { return; } - this->m_recObs->swapSteps(*this->m_recIns[row], *this->m_recIns[row+1]); + this->m_recObs->m_instructions.swap(*this->m_recIns[row], *this->m_recIns[row+1]); // After updating the model, this is the simplest way to update the display this->setRecipe(this->m_recObs); @@ -248,7 +248,7 @@ void BrewDayScrollWidget::setRecipe(Recipe* rec) { this->m_recObs = rec; connect(this->m_recObs, &Recipe::changed, this, &BrewDayScrollWidget::acceptChanges); - m_recIns = this->m_recObs->steps(); + m_recIns = this->m_recObs->m_instructions.items(); for (auto ins : m_recIns) { connect(ins.get(), &Instruction::changed, this, &BrewDayScrollWidget::acceptInsChanges); } @@ -288,7 +288,7 @@ void BrewDayScrollWidget::insertInstruction() { lineEdit_name->clear(); pos = qBound(1, pos, this->m_recIns.size()); - this->m_recObs->insertStep(instruction, pos); + this->m_recObs->m_instructions.insert(instruction, pos); // After updating the model, this is the simplest way to update the display this->setRecipe(this->m_recObs); @@ -298,12 +298,12 @@ void BrewDayScrollWidget::insertInstruction() { } void BrewDayScrollWidget::acceptChanges(QMetaProperty prop, QVariant /*value*/) { - if (m_recObs && QString(prop.name()) == PropertyNames::SteppedOwnerBase::steps) { + if (m_recObs && QString(prop.name()) == PropertyNames::Recipe::instructions) { // An instruction has been added or deleted, so update internal list. for (auto ins : m_recIns ) { disconnect(ins.get(), nullptr, this, nullptr); } - m_recIns = this->m_recObs->steps(); // Already sorted by instruction numbers. + m_recIns = this->m_recObs->m_instructions.items(); // Already sorted by instruction numbers. for (auto ins : m_recIns ) { connect(ins.get(), &Instruction::changed, this, &BrewDayScrollWidget::acceptInsChanges); } @@ -438,7 +438,7 @@ QString BrewDayScrollWidget::buildInstructionTable() { .arg(tr("Time")) .arg(tr("Step")); - auto instructions = this->m_recObs->steps(); + auto instructions = this->m_recObs->m_instructions.items(); auto mashSteps = this->m_recObs->mash()->mashSteps(); int size = instructions.size(); for (int i = 0; i < size; ++i ) { diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp index c04a2670..e2fe5408 100644 --- a/src/MainWindow.cpp +++ b/src/MainWindow.cpp @@ -1976,7 +1976,7 @@ void MainWindow::showChanges(QMetaProperty* prop) { if (this->pimpl->m_recipeObs->boil() && (updateAll || propName == PropertyNames::Recipe::boil || - propName == PropertyNames::SteppedOwnerBase::steps)) { + propName == PropertyNames::StepOwnerBase::steps)) { this->pimpl->m_boilStepTableModel->setBoil(this->pimpl->m_recipeObs->boil()); } // See if we need to change the fermentation in the table. diff --git a/src/RecipeFormatter.cpp b/src/RecipeFormatter.cpp index ea6b31bd..f06fe403 100644 --- a/src/RecipeFormatter.cpp +++ b/src/RecipeFormatter.cpp @@ -924,7 +924,7 @@ class RecipeFormatter::impl { return ""; } - auto instructions = this->rec->steps(); + auto instructions = this->rec->instructions(); int size = instructions.size(); if ( size < 1 ) { return ""; @@ -952,7 +952,7 @@ class RecipeFormatter::impl { QStringList num, text; - auto instructions = rec->steps(); + auto instructions = rec->instructions(); int size = instructions.size(); if ( size > 0 ) { for (int ii = 0; ii < size; ++ii) { diff --git a/src/WaterDialog.h b/src/WaterDialog.h index 5a04d674..58ed4bd0 100644 --- a/src/WaterDialog.h +++ b/src/WaterDialog.h @@ -30,6 +30,7 @@ #include "ui_waterDialog.h" #include "measurement/Unit.h" +#include "model/Salt.h" #include "model/Water.h" class WaterListModel; @@ -37,7 +38,6 @@ class WaterSortFilterProxyModel; class WaterEditor; class RecipeAdjustmentSaltTableModel; class RecipeAdjustmentSaltItemDelegate; -class Salt; /*! * \class WaterDialog diff --git a/src/database/ObjectStoreTyped.cpp b/src/database/ObjectStoreTyped.cpp index c894e962..548b7d61 100644 --- a/src/database/ObjectStoreTyped.cpp +++ b/src/database/ObjectStoreTyped.cpp @@ -306,10 +306,10 @@ namespace { // NB: MashSteps don't have folders, as each one is owned by a Mash {ObjectStore::FieldType::Double, "end_temp_c" , PropertyNames:: Step::endTemp_c }, {ObjectStore::FieldType::Double, "infuse_temp_c" , PropertyNames:: MashStep::infuseTemp_c }, - {ObjectStore::FieldType::Int , "mash_id" , PropertyNames::SteppedBase::ownerId , &PRIMARY_TABLE}, + {ObjectStore::FieldType::Int , "mash_id" , PropertyNames::EnumeratedBase::ownerId , &PRIMARY_TABLE}, {ObjectStore::FieldType::Enum , "mstype" , PropertyNames:: MashStep::type , &MashStep::typeStringMapping}, {ObjectStore::FieldType::Double, "ramp_time_mins" , PropertyNames:: StepBase::rampTime_mins }, - {ObjectStore::FieldType::Int , "step_number" , PropertyNames::SteppedBase::stepNumber }, + {ObjectStore::FieldType::Int , "step_number" , PropertyNames::EnumeratedBase::stepNumber }, {ObjectStore::FieldType::Double, "step_temp_c" , PropertyNames:: StepBase::startTemp_c }, {ObjectStore::FieldType::Double, "step_time_mins" , PropertyNames:: StepBase::stepTime_mins }, // Now we support BeerJSON, amount_l unifies and replaces infuseAmount_l and decoctionAmount_l @@ -360,8 +360,8 @@ namespace { {ObjectStore::FieldType::Double, "start_temp_c" , PropertyNames:: StepBase::startTemp_c }, {ObjectStore::FieldType::Double, "end_temp_c" , PropertyNames:: Step::endTemp_c }, {ObjectStore::FieldType::Double, "ramp_time_mins" , PropertyNames:: StepBase::rampTime_mins }, - {ObjectStore::FieldType::Int , "step_number" , PropertyNames:: SteppedBase::stepNumber }, - {ObjectStore::FieldType::Int , "boil_id" , PropertyNames:: SteppedBase::ownerId , &PRIMARY_TABLE}, + {ObjectStore::FieldType::Int , "step_number" , PropertyNames:: EnumeratedBase::stepNumber }, + {ObjectStore::FieldType::Int , "boil_id" , PropertyNames:: EnumeratedBase::ownerId , &PRIMARY_TABLE}, {ObjectStore::FieldType::String, "description" , PropertyNames:: Step::description }, {ObjectStore::FieldType::Double, "start_acidity_ph", PropertyNames:: Step::startAcidity_pH}, {ObjectStore::FieldType::Double, "end_acidity_ph" , PropertyNames:: Step::endAcidity_pH }, @@ -410,8 +410,8 @@ namespace { {ObjectStore::FieldType::Double, "step_time_mins" , PropertyNames:: StepBase::stepTime_mins }, {ObjectStore::FieldType::Double, "start_temp_c" , PropertyNames:: StepBase::startTemp_c }, {ObjectStore::FieldType::Double, "end_temp_c" , PropertyNames:: Step::endTemp_c }, - {ObjectStore::FieldType::Int , "step_number" , PropertyNames:: SteppedBase::stepNumber }, - {ObjectStore::FieldType::Int , "fermentation_id" , PropertyNames:: SteppedBase::ownerId , &PRIMARY_TABLE}, + {ObjectStore::FieldType::Int , "step_number" , PropertyNames:: EnumeratedBase::stepNumber }, + {ObjectStore::FieldType::Int , "fermentation_id" , PropertyNames:: EnumeratedBase::ownerId , &PRIMARY_TABLE}, {ObjectStore::FieldType::String, "description" , PropertyNames:: Step::description }, {ObjectStore::FieldType::Double, "start_acidity_ph", PropertyNames:: Step::startAcidity_pH}, {ObjectStore::FieldType::Double, "end_acidity_ph" , PropertyNames:: Step:: endAcidity_pH}, @@ -846,8 +846,8 @@ namespace { {ObjectStore::FieldType::String, "name" , PropertyNames::NamedEntity::name }, {ObjectStore::FieldType::Bool , "display" , PropertyNames::NamedEntity::display }, {ObjectStore::FieldType::Bool , "deleted" , PropertyNames::NamedEntity::deleted }, - {ObjectStore::FieldType::Int , "recipe_id" , PropertyNames::SteppedBase::ownerId , &PRIMARY_TABLE}, - {ObjectStore::FieldType::Int , "step_number" , PropertyNames::SteppedBase::stepNumber}, + {ObjectStore::FieldType::Int , "recipe_id" , PropertyNames::EnumeratedBase::ownerId , &PRIMARY_TABLE}, + {ObjectStore::FieldType::Int , "step_number" , PropertyNames::EnumeratedBase::stepNumber}, {ObjectStore::FieldType::String, "directions" , PropertyNames::Instruction::directions}, {ObjectStore::FieldType::Bool , "has_timer" , PropertyNames::Instruction::hasTimer }, {ObjectStore::FieldType::String, "timer_value" , PropertyNames::Instruction::timerValue}, diff --git a/src/model/Boil.h b/src/model/Boil.h index db1623c8..cdbd0408 100755 --- a/src/model/Boil.h +++ b/src/model/Boil.h @@ -72,7 +72,7 @@ class Boil : public NamedEntity, STEP_OWNER_COMMON_DECL(Boil, boil) // See model/FolderBase.h for info, getters and setters for these properties Q_PROPERTY(QString folder READ folder WRITE setFolder ) - // See model/SteppedOwnerBase.h for info, getters and setters for these properties + // See model/StepOwnerBase.h for info, getters and setters for these properties Q_PROPERTY(QList> steps READ steps WRITE setSteps STORED false) Q_PROPERTY(unsigned int numSteps READ numSteps STORED false) diff --git a/src/model/BoilStep.h b/src/model/BoilStep.h index ab9bd945..e7687b3b 100755 --- a/src/model/BoilStep.h +++ b/src/model/BoilStep.h @@ -46,7 +46,7 @@ class BoilStep : public StepExtended, public StepBase * * Brewtarget is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License @@ -13,8 +13,8 @@ * You should have received a copy of the GNU General Public License along with this program. If not, see * . ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌*/ -#ifndef MODEL_STEPPEDBASE_H -#define MODEL_STEPPEDBASE_H +#ifndef MODEL_ENUMERATEDBASE_H +#define MODEL_ENUMERATEDBASE_H #pragma once #include "database/ObjectStoreWrapper.h" @@ -25,7 +25,7 @@ //====================================================================================================================== //========================================== Start of property name constants ========================================== // See comment in model/NamedEntity.h -#define AddPropertyName(property) namespace PropertyNames::SteppedBase { inline BtStringConst const property{#property}; } +#define AddPropertyName(property) namespace PropertyNames::EnumeratedBase { inline BtStringConst const property{#property}; } AddPropertyName(ownerId ) AddPropertyName(stepNumber) #undef AddPropertyName @@ -45,39 +45,33 @@ AddPropertyName(stepNumber) * corresponding \c Instruction objects). * - The \c Instruction objects belonging to a given \c Recipe have an ordering * - * So we pull out these more fundamental properties into \c SteppedBase and \c SteppedOwnerBase, as the following - * \b partial inheritance diagram shows: + * So we pull out these more fundamental properties into \c EnumeratedBase, as the following \b partial inheritance + * diagram shows: * - * SteppedBase SteppedOwnerBase - * / \ / \ - * StepBase Instruction StepOwnerBase Recipe - * / | \ / | \ - * / | \ / | \ - * / | \ / | \ - * MashStep BoilStep FermentationStep Mash Boil Fermentation + * EnumeratedBase + * / \ + * StepBase Instruction + * / | \ + * / | \ + * / | \ + * MashStep BoilStep FermentationStep * * This gives enough of what we need, for now. We would need to rethink a bit if we had one type of object * owning more than one type of "steps". * - * NOTE that the "stepped" concept does not apply to other things "owned" by Recipe, such as \c RecipeAdditionHop - * or \c BrewNote, because they do not have the same need of pure ordering. There is an implicit ordering by - * addition time or date, but it is not necessary or desirable to be more precise than this. You will often have - * two hop additions that happen at the same time, for instance. See \c OwnedSet for the logic for dealing with - * these. - * - * Directly derived classes need to include STEPPED_COMMON_DECL and STEPPED_COMMON_CODE in the obvious places + * Directly derived classes need to include ENUMERATED_COMMON_DECL and ENUMERATED_COMMON_CODE in the obvious places * (either their own macros for \c StepBase or in the header and implementation file respectively for * \c Instruction). */ -template class SteppedPhantom; +template class EnumeratedPhantom; template -class SteppedBase : public CuriouslyRecurringTemplateBase { +class EnumeratedBase : public CuriouslyRecurringTemplateBase { protected: // Note that, because this is static, it cannot be initialised inside the class definition static TypeLookup const typeLookup; //! Non-virtual equivalent of isEqualTo - bool doIsEqualTo(SteppedBase const & other) const { + bool doIsEqualTo(EnumeratedBase const & other) const { return ( Utils::AutoCompare(this->m_ownerId , other.m_ownerId ) && Utils::AutoCompare(this->m_stepNumber, other.m_stepNumber) @@ -85,24 +79,24 @@ class SteppedBase : public CuriouslyRecurringTemplateBasem_ownerId ; } @@ -112,7 +106,7 @@ class SteppedBase : public CuriouslyRecurringTemplateBasem_ownerId = val; - this->derived().propagatePropertyChange(PropertyNames::SteppedBase::ownerId, false); + this->derived().propagatePropertyChange(PropertyNames::EnumeratedBase::ownerId, false); return; } @@ -124,10 +118,10 @@ class SteppedBase : public CuriouslyRecurringTemplateBasederived().setAndNotify(PropertyNames::SteppedBase::stepNumber, this->m_stepNumber, val); + this->derived().setAndNotify(PropertyNames::EnumeratedBase::stepNumber, this->m_stepNumber, val); } else { this->m_stepNumber = val; - this->derived().propagatePropertyChange(PropertyNames::SteppedBase::stepNumber, false); + this->derived().propagatePropertyChange(PropertyNames::EnumeratedBase::stepNumber, false); } return; } @@ -149,12 +143,12 @@ class SteppedBase : public CuriouslyRecurringTemplateBase> ownedBy(std::shared_ptr owner) { - // We already wrote all the logic in SteppedOwnerBase. - return owner->steps(); - } +// static QList> ownedBy(std::shared_ptr owner) { +// // We already wrote all the logic in StepOwnerBase. +// return owner->steps(); +// } ObjectStore & doGetObjectStoreTypedInstance() const { return ObjectStoreTyped::getInstance(); @@ -170,22 +164,22 @@ class SteppedBase : public CuriouslyRecurringTemplateBase -TypeLookup const SteppedBase::typeLookup { - "SteppedBase", +TypeLookup const EnumeratedBase::typeLookup { + "EnumeratedBase", { // // See comment in model/IngredientAmount.h for why we can't use the PROPERTY_TYPE_LOOKUP_ENTRY or // PROPERTY_TYPE_LOOKUP_ENTRY_NO_MV macros here. // - {&PropertyNames::SteppedBase::ownerId, - TypeInfo::construct::m_ownerId)>( - PropertyNames::SteppedBase::ownerId, - TypeLookupOf::m_ownerId)>::value + {&PropertyNames::EnumeratedBase::ownerId, + TypeInfo::construct::m_ownerId)>( + PropertyNames::EnumeratedBase::ownerId, + TypeLookupOf::m_ownerId)>::value )}, - {&PropertyNames::SteppedBase::stepNumber, - TypeInfo::construct::m_stepNumber)>( - PropertyNames::SteppedBase::stepNumber, - TypeLookupOf::m_stepNumber)>::value, + {&PropertyNames::EnumeratedBase::stepNumber, + TypeInfo::construct::m_stepNumber)>( + PropertyNames::EnumeratedBase::stepNumber, + TypeLookupOf::m_stepNumber)>::value, NonPhysicalQuantity::OrdinalNumeral )} }, @@ -198,7 +192,7 @@ TypeLookup const SteppedBase::typeLookup { * include this in their header file, right after Q_OBJECT. Concrete derived classes also need to include the * following block (see comment in model/StepBase.h for why): * - * // See model/SteppedBase.h for info, getters and setters for these properties + * // See model/EnumeratedBase.h for info, getters and setters for these properties * Q_PROPERTY(int ownerId READ ownerId WRITE setOwnerId ) * Q_PROPERTY(int stepNumber READ stepNumber WRITE setStepNumber) * @@ -216,9 +210,9 @@ TypeLookup const SteppedBase::typeLookup { * In older versions of Qt, we didn't used to be able to put Q_PROPERTY inside our own macro, but, thankfully, * the Qt MOC (meta object compiler) now expands "normal" macros before processing its own "special" ones. */ -#define STEPPED_COMMON_DECL(Derived, Owner) \ +#define ENUMERATED_COMMON_DECL(Derived, Owner) \ /* This allows StepBase to call protected and private members of Derived */ \ - friend class SteppedBase; \ + friend class EnumeratedBase; \ public: \ /* This alias makes it easier to template a number of functions */ \ /* that are essentially the same for all "stepped" classes. */ \ @@ -232,7 +226,7 @@ TypeLookup const SteppedBase::typeLookup { * \brief Derived classes should (either directly or via inclusion in an intermediate class's equivalent macro) include * this in their implementation file. */ -#define STEPPED_COMMON_CODE(Derived) \ +#define ENUMERATED_COMMON_CODE(Derived) \ ObjectStore & Derived::getObjectStoreTypedInstance() const { return this->doGetObjectStoreTypedInstance(); } \ diff --git a/src/model/Fermentation.h b/src/model/Fermentation.h index 316a91aa..bb6ffef2 100755 --- a/src/model/Fermentation.h +++ b/src/model/Fermentation.h @@ -56,7 +56,7 @@ class Fermentation : public NamedEntity, STEP_OWNER_COMMON_DECL(Fermentation, fermentation) // See model/FolderBase.h for info, getters and setters for these properties Q_PROPERTY(QString folder READ folder WRITE setFolder ) - // See model/SteppedOwnerBase.h for info, getters and setters for these properties + // See model/StepOwnerBase.h for info, getters and setters for these properties Q_PROPERTY(QList> steps READ steps WRITE setSteps STORED false) Q_PROPERTY(unsigned int numSteps READ numSteps STORED false) diff --git a/src/model/FermentationStep.h b/src/model/FermentationStep.h index 9e189007..e66a25fd 100755 --- a/src/model/FermentationStep.h +++ b/src/model/FermentationStep.h @@ -47,7 +47,7 @@ class FermentationStep : public StepExtended, public StepBasem_hasTimer == rhs.m_hasTimer && this->m_timerValue == rhs.m_timerValue && // Parent classes have to be equal too - this->SteppedBase::doIsEqualTo(rhs) + this->EnumeratedBase::doIsEqualTo(rhs) ); } @@ -48,12 +48,12 @@ TypeLookup const Instruction::typeLookup { }, // Parent class lookup {&NamedEntity::typeLookup, - std::addressof(SteppedBase::typeLookup)} + std::addressof(EnumeratedBase::typeLookup)} }; Instruction::Instruction(QString name) : NamedEntity {name }, - SteppedBase{}, + EnumeratedBase{}, m_directions {"" }, m_hasTimer {false}, m_timerValue {"" }, @@ -66,7 +66,7 @@ Instruction::Instruction(QString name) : Instruction::Instruction(NamedParameterBundle const & namedParameterBundle) : NamedEntity{namedParameterBundle}, - SteppedBase{namedParameterBundle}, + EnumeratedBase{namedParameterBundle}, SET_REGULAR_FROM_NPB (m_directions, namedParameterBundle, PropertyNames::Instruction::directions), SET_REGULAR_FROM_NPB (m_hasTimer , namedParameterBundle, PropertyNames::Instruction::hasTimer ), SET_REGULAR_FROM_NPB (m_timerValue, namedParameterBundle, PropertyNames::Instruction::timerValue), @@ -79,7 +79,7 @@ Instruction::Instruction(NamedParameterBundle const & namedParameterBundle) : Instruction::Instruction(Instruction const & other) : NamedEntity {other}, - SteppedBase{other}, + EnumeratedBase{other}, m_directions {other.m_directions}, m_hasTimer {other.m_hasTimer }, m_timerValue {other.m_timerValue}, @@ -151,5 +151,5 @@ bool operator<(Instruction & lhs, Instruction & rhs) { return lhs.stepNumber() < rhs.stepNumber(); } -// Insert boiler-plate wrapper functions that call down to SteppedBase -STEPPED_COMMON_CODE(Instruction) +// Insert boiler-plate wrapper functions that call down to EnumeratedBase +ENUMERATED_COMMON_CODE(Instruction) diff --git a/src/model/Instruction.h b/src/model/Instruction.h index 124f3000..4279cf68 100644 --- a/src/model/Instruction.h +++ b/src/model/Instruction.h @@ -26,7 +26,7 @@ #include #include "model/NamedEntity.h" -#include "model/SteppedBase.h" +#include "model/EnumeratedBase.h" //====================================================================================================================== //========================================== Start of property name constants ========================================== @@ -53,14 +53,14 @@ class Recipe; * BeerXML files, because we can, but TBD whether this is possible with BeerJSON. * * NB: We do not inherit from \c OwnedByRecipe, because doing so would duplicate part of what we get from - * \c SteppedBase. + * \c EnumeratedBase. */ class Instruction : public NamedEntity, - public SteppedBase { + public EnumeratedBase { Q_OBJECT - STEPPED_COMMON_DECL(Instruction, Recipe) - // See model/SteppedBase.h for info, getters and setters for these properties + ENUMERATED_COMMON_DECL(Instruction, Recipe) + // See model/EnumeratedBase.h for info, getters and setters for these properties Q_PROPERTY(int ownerId READ ownerId WRITE setOwnerId ) Q_PROPERTY(int stepNumber READ stepNumber WRITE setStepNumber) diff --git a/src/model/Mash.h b/src/model/Mash.h index e4929055..35d57908 100644 --- a/src/model/Mash.h +++ b/src/model/Mash.h @@ -72,7 +72,7 @@ class Mash : public NamedEntity, STEP_OWNER_COMMON_DECL(Mash, mash) // See model/FolderBase.h for info, getters and setters for these properties Q_PROPERTY(QString folder READ folder WRITE setFolder ) - // See model/SteppedOwnerBase.h for info, getters and setters for these properties + // See model/StepOwnerBase.h for info, getters and setters for these properties Q_PROPERTY(QList> steps READ steps WRITE setSteps STORED false) Q_PROPERTY(unsigned int numSteps READ numSteps STORED false) diff --git a/src/model/MashStep.h b/src/model/MashStep.h index 728586ed..eba4a753 100644 --- a/src/model/MashStep.h +++ b/src/model/MashStep.h @@ -59,7 +59,7 @@ class MashStep : public Step, public StepBase { Q_OBJECT STEP_COMMON_DECL(Mash, MashStepOptions) - // See model/SteppedBase.h for info, getters and setters for these properties + // See model/EnumeratedBase.h for info, getters and setters for these properties Q_PROPERTY(int ownerId READ ownerId WRITE setOwnerId ) Q_PROPERTY(int stepNumber READ stepNumber WRITE setStepNumber) // See model/StepBase.h for info, getters and setters for these properties diff --git a/src/model/Recipe.cpp b/src/model/Recipe.cpp index 37754e0c..a1d6fd6d 100644 --- a/src/model/Recipe.cpp +++ b/src/model/Recipe.cpp @@ -45,6 +45,7 @@ #include "measurement/Unit.h" #include "model/Boil.h" #include "model/BoilStep.h" +#include "model/BrewNote.h" #include "model/Equipment.h" #include "model/Fermentable.h" #include "model/Fermentation.h" @@ -55,7 +56,6 @@ #include "model/MashStep.h" #include "model/Misc.h" #include "model/NamedParameterBundle.h" -#include "model/OwnedSet.h" #include "model/RecipeAdditionFermentable.h" #include "model/RecipeAdditionHop.h" #include "model/RecipeAdditionMisc.h" @@ -153,7 +153,6 @@ namespace { template<> BtStringConst const & Recipe::propertyNameFor() { return PropertyNames::Recipe::boilId ; } template<> BtStringConst const & Recipe::propertyNameFor() { return PropertyNames::Recipe::equipmentId ; } template<> BtStringConst const & Recipe::propertyNameFor() { return PropertyNames::Recipe::fermentationId ; } -///template<> BtStringConst const & Recipe::propertyNameFor() { return PropertyNames::Recipe::instructionIds ; } template<> BtStringConst const & Recipe::propertyNameFor() { return PropertyNames::Recipe::mashId ; } template<> BtStringConst const & Recipe::propertyNameFor