Skip to content

Commit

Permalink
Fix for Brewtarget#785
Browse files Browse the repository at this point in the history
  • Loading branch information
Matt Young committed Jan 9, 2024
1 parent 268a211 commit 58f4d1c
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 31 deletions.
12 changes: 7 additions & 5 deletions src/RecipeExtrasWidget.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* RecipeExtrasWidget.cpp is part of Brewtarget, and is Copyright the following
* authors 2009-2023
* authors 2009-2024
* - Matt Young <mfsy@yahoo.com>
* - Mik Firestone <mikfire@gmail.com>
* - Peter Buelow <goballstate@gmail.com>
Expand Down Expand Up @@ -117,7 +117,9 @@ void RecipeExtrasWidget::updateTasteRating() {

void RecipeExtrasWidget::updatePrimaryAge() {
if (!this->recipe) { return;}
MainWindow::instance().doOrRedoUpdate(*recipe, PropertyNames::Recipe::primaryAge_days, lineEdit_primaryAge->toCanonical().quantity(), tr("Change Primary Age"));
// See comment in model/Recipe.cpp for why age_days, primaryAge_days, secondaryAge_days, tertiaryAge_days properties
// are dimensionless
MainWindow::instance().doOrRedoUpdate(*recipe, PropertyNames::Recipe::primaryAge_days, lineEdit_primaryAge->getNonOptValueAs<double>(), tr("Change Primary Age"));
}

void RecipeExtrasWidget::updatePrimaryTemp() {
Expand All @@ -127,7 +129,7 @@ void RecipeExtrasWidget::updatePrimaryTemp() {

void RecipeExtrasWidget::updateSecondaryAge() {
if (!this->recipe) { return;}
MainWindow::instance().doOrRedoUpdate(*recipe, PropertyNames::Recipe::secondaryAge_days, lineEdit_secAge->toCanonical().quantity(), tr("Change Secondary Age"));
MainWindow::instance().doOrRedoUpdate(*recipe, PropertyNames::Recipe::secondaryAge_days, lineEdit_secAge->getNonOptValueAs<double>(), tr("Change Secondary Age"));
}

void RecipeExtrasWidget::updateSecondaryTemp() {
Expand All @@ -137,7 +139,7 @@ void RecipeExtrasWidget::updateSecondaryTemp() {

void RecipeExtrasWidget::updateTertiaryAge() {
if (!this->recipe) { return;}
MainWindow::instance().doOrRedoUpdate(*recipe, PropertyNames::Recipe::tertiaryAge_days, lineEdit_tertAge->toCanonical().quantity(), tr("Change Tertiary Age"));
MainWindow::instance().doOrRedoUpdate(*recipe, PropertyNames::Recipe::tertiaryAge_days, lineEdit_tertAge->getNonOptValueAs<double>(), tr("Change Tertiary Age"));
}

void RecipeExtrasWidget::updateTertiaryTemp() {
Expand All @@ -147,7 +149,7 @@ void RecipeExtrasWidget::updateTertiaryTemp() {

void RecipeExtrasWidget::updateAge() {
if (!this->recipe) { return;}
MainWindow::instance().doOrRedoUpdate(*recipe, PropertyNames::Recipe::age_days, lineEdit_age->toCanonical().quantity(), tr("Change Age"));
MainWindow::instance().doOrRedoUpdate(*recipe, PropertyNames::Recipe::age_days, lineEdit_age->getNonOptValueAs<double>(), tr("Change Age"));
}

void RecipeExtrasWidget::updateAgeTemp() {
Expand Down
58 changes: 35 additions & 23 deletions src/database/ObjectStore.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* database/ObjectStore.cpp is part of Brewtarget, and is copyright the following
* authors 2021-2023:
* authors 2021-2024:
* • Matt Young <mfsy@yahoo.com>
*
* Brewtarget is free software: you can redistribute it and/or modify
Expand Down Expand Up @@ -695,46 +695,58 @@ class ObjectStore::impl {
/// propertyValue.typeName() << "(" << propertyType << ") in column" << fieldDefn.columnName <<
/// ". This is a known ugliness that we intend to fix one day.";
} else {
// It's not a known exception, so it's a coding error. However, we may still be able to recover.
// If we are expecting a boolean and we get a string holding "true" or "false" etc, then we know what to
// do.
//
// It's not a known exception, so it's a coding error. However, we can recover in the following cases:
// - If we are expecting a boolean and we get a string holding "true" or "false" etc, then we know
// what to do.
// - If we are expecting an int and we get a double then we can just ignore the non-integer part of
// the number.
// - If we are expecting an double and we got an int then we can just upcast it.
//
bool recovered = false;
QVariant readPropertyValue = propertyValue;
if (propertyType == QMetaType::QString && fieldDefn.fieldType == ObjectStore::FieldType::Bool) {
// We'll take any reasonable string representation of true/false. For the moment, at least, I'm not
// worrying about optional fields here. I think it's pretty rare, if ever, that we'd want an optional
// boolean.
QString propertyAsLcString = propertyValue.toString().trimmed().toLower();
bool interpretedValue = false;
if (propertyAsLcString == "true" || propertyAsLcString == "t" || propertyAsLcString == "1") {
recovered = true;
interpretedValue = true;
propertyValue = QVariant(true);
} else if (propertyAsLcString == "false" || propertyAsLcString == "f" || propertyAsLcString == "0") {
recovered = true;
interpretedValue = false;
}
if (recovered) {
qWarning() <<
Q_FUNC_INFO << "Recovered from unexpected type #" << propertyType << "=" <<
propertyValue.typeName() << "in QVariant for property" << fieldDefn.propertyName <<
", field type" << fieldDefn.fieldType << ", value" << propertyValue << ", table" <<
primaryTable.tableName << ", column" << fieldDefn.columnName << ". Interpreted value as" <<
interpretedValue;
// Now we overwrite the supplied variant with what we worked out it should be, so processing can
// continue.
propertyValue = QVariant(interpretedValue);
propertyValue = QVariant(false);
}
} else if (propertyType == QMetaType::Double && fieldDefn.fieldType == ObjectStore::FieldType::Int) {
propertyValue = QVariant(propertyValue.toInt(&recovered));
} else if (propertyType == QMetaType::Int && fieldDefn.fieldType == ObjectStore::FieldType::Double) {
propertyValue = QVariant(propertyValue.toDouble(&recovered));
}
if (!recovered) {
if (recovered) {
qWarning() <<
Q_FUNC_INFO << "Recovered from unexpected type #" << propertyType << "=" <<
readPropertyValue.typeName() << "in QVariant for property" << fieldDefn.propertyName <<
", field type" << fieldDefn.fieldType << ", value" << readPropertyValue << ", table" <<
primaryTable.tableName << ", column" << fieldDefn.columnName << ". Interpreted value as" <<
propertyValue;
} else {
//
// Even in the case where we do not have a reasonable way to interpret the data in this column, we
// should probably NOT terminate the program. We are still discovering lots of cases where folks
// using the software have old Brewtarget databases with quirky values in some columns. It's better
// from a user point of view that the software carries on working even if some (hopefully) obscure
// field could not be read from the DB.
//
qCritical() <<
Q_FUNC_INFO << "Unexpected type #" << propertyType << "=" << propertyValue.typeName() <<
"in QVariant for property" << fieldDefn.propertyName << ", field type" << fieldDefn.fieldType <<
", value" << propertyValue << ", table" << primaryTable.tableName << ", column" <<
fieldDefn.columnName;
qCritical().noquote() << Q_FUNC_INFO << "Call stack is:" << Logging::getStackTrace();
// Stop here on debug build
Q_ASSERT(false);
// If, during development or debugging, you want to have the program stop when it cannot interpret
// one of the DB fields, then uncomment the following two lines.
/// qCritical().noquote() << Q_FUNC_INFO << "Call stack is:" << Logging::getStackTrace();
/// Q_ASSERT(false);
}

}
}
}
Expand Down
9 changes: 8 additions & 1 deletion src/widgets/SmartField.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ class SmartField::impl {
Measurement::Amount toCanonical(QString const & enteredText, SmartAmounts::ScaleInfo previousScaleInfo) {
Q_ASSERT(this->m_initialised);

// It's a coding error to call this for a NonPhysicalQuantity
// It's a coding error to call this for a NonPhysicalQuantity. (Instead call getNonOptValueAs<double> or
// similar.)
Q_ASSERT(!std::holds_alternative<NonPhysicalQuantity>(*this->m_typeInfo->fieldType));
Q_ASSERT(this->m_currentPhysicalQuantity);

Expand Down Expand Up @@ -401,6 +402,12 @@ SmartAmounts::ScaleInfo SmartField::getScaleInfo() const {
return SmartAmounts::ScaleInfo{this->pimpl->m_fixedDisplayUnit->getUnitSystem().systemOfMeasurement, std::nullopt};
}

// Only need next bit for debugging!
// if (std::holds_alternative<NonPhysicalQuantity>(*this->pimpl->m_typeInfo->fieldType)) {
// qCritical() << Q_FUNC_INFO << this->pimpl->m_typeInfo;
// qCritical().noquote() << Q_FUNC_INFO << "Stack trace:" << Logging::getStackTrace();
// }

Q_ASSERT(!std::holds_alternative<NonPhysicalQuantity>(*this->pimpl->m_typeInfo->fieldType));
return SmartAmounts::getScaleInfo(this->pimpl->m_editorName,
this->pimpl->m_fieldName,
Expand Down
4 changes: 2 additions & 2 deletions src/widgets/SmartField.h
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* widgets/SmartField.h is part of Brewtarget, and is copyright the following authors 2009-2023:
* widgets/SmartField.h is part of Brewtarget, and is copyright the following authors 2009-2024:
* • Brian Rower <brian.rower@gmail.com>
* • Mark de Wever <koraq@xs4all.nl>
* • Matt Young <mfsy@yahoo.com>
Expand Down Expand Up @@ -62,7 +62,7 @@ class TypeInfo;
* / SmartField \
* QLabel / \ \
* / \ / \ \
* SmartLabel SmartDigitWidget SmartField
* SmartLabel SmartDigitWidget SmartLineEdit
*
* A number of helper functions exist in the \c SmartAmounts namespace.
*
Expand Down

0 comments on commit 58f4d1c

Please sign in to comment.