From 4d9ee1626d6de27d1ae1436bca0000ca14ecfc02 Mon Sep 17 00:00:00 2001 From: Klaus Basan Date: Thu, 3 Aug 2017 01:03:07 +0200 Subject: [PATCH] Ref T120, refactoring of modulator related classes * avoid virtual functions in modulator related classes * removed unused isDefaultValue() * using null values (which was not present when class was written) * removed destructor --- src/blackmisc/aviation/adfsystem.h | 30 +++++++---------- src/blackmisc/aviation/comsystem.cpp | 18 +++-------- src/blackmisc/aviation/comsystem.h | 29 +++++++++-------- src/blackmisc/aviation/modulator.cpp | 45 ++++++-------------------- src/blackmisc/aviation/modulator.h | 30 ++++------------- src/blackmisc/aviation/navsystem.h | 45 ++++++++++++-------------- src/blackmisc/aviation/transponder.cpp | 1 - 7 files changed, 66 insertions(+), 132 deletions(-) diff --git a/src/blackmisc/aviation/adfsystem.h b/src/blackmisc/aviation/adfsystem.h index a4d3cceb7..a3ebc0fa8 100644 --- a/src/blackmisc/aviation/adfsystem.h +++ b/src/blackmisc/aviation/adfsystem.h @@ -21,7 +21,6 @@ namespace BlackMisc { namespace Aviation { - //! ADF system ("for NDBs") class BLACKMISC_EXPORT CAdfSystem : public CModulator, @@ -40,8 +39,9 @@ namespace BlackMisc CAdfSystem() = default; //! Constructor - CAdfSystem(const QString &name, const PhysicalQuantities::CFrequency &activeFrequency, const PhysicalQuantities::CFrequency &standbyFrequency = CModulator::FrequencyNotSet()): - CModulator(name, activeFrequency, standbyFrequency == CModulator::FrequencyNotSet() ? activeFrequency : standbyFrequency) + CAdfSystem(const QString &name, const PhysicalQuantities::CFrequency &activeFrequency, + const PhysicalQuantities::CFrequency &standbyFrequency = { 0, BlackMisc::PhysicalQuantities::CFrequencyUnit::nullUnit() }): + CModulator(name, activeFrequency, standbyFrequency.isNull() ? activeFrequency : standbyFrequency) { } //! Valid aviation frequency? @@ -58,9 +58,10 @@ namespace BlackMisc } //! ADF1 unit - static CAdfSystem GetAdf1System(const PhysicalQuantities::CFrequency &activeFrequency, const PhysicalQuantities::CFrequency &standbyFrequency = CModulator::FrequencyNotSet()) + static CAdfSystem GetAdf1System(const PhysicalQuantities::CFrequency &activeFrequency, + const PhysicalQuantities::CFrequency &standbyFrequency = { 0, BlackMisc::PhysicalQuantities::CFrequencyUnit::nullUnit() }) { - return CAdfSystem(CModulator::NameCom1(), activeFrequency, standbyFrequency == CModulator::FrequencyNotSet() ? activeFrequency : standbyFrequency); + return CAdfSystem(CModulator::NameCom1(), activeFrequency, standbyFrequency.isNull() ? activeFrequency : standbyFrequency); } //! ADF2 unit @@ -70,23 +71,14 @@ namespace BlackMisc } //! ADF2 unit - static CAdfSystem GetAdf2System(const PhysicalQuantities::CFrequency &activeFrequency, const PhysicalQuantities::CFrequency &standbyFrequency = CModulator::FrequencyNotSet()) + static CAdfSystem GetAdf2System(const PhysicalQuantities::CFrequency &activeFrequency, + const PhysicalQuantities::CFrequency &standbyFrequency = { 0, BlackMisc::PhysicalQuantities::CFrequencyUnit::nullUnit() }) { - return CAdfSystem(CModulator::NameCom2(), activeFrequency, standbyFrequency == CModulator::FrequencyNotSet() ? activeFrequency : standbyFrequency); - } - - protected: - //! \copydoc CModulator::validValues - virtual bool validValues() const override - { - if (this->isDefaultValue()) return true; // special case - return - this->isValidFrequency(this->getFrequencyActive()) && - this->isValidFrequency(this->getFrequencyStandby()); + return CAdfSystem(CModulator::NameCom2(), activeFrequency, standbyFrequency.isNull() ? activeFrequency : standbyFrequency); } }; - } -} + } // ns +} // ns Q_DECLARE_METATYPE(BlackMisc::Aviation::CAdfSystem) diff --git a/src/blackmisc/aviation/comsystem.cpp b/src/blackmisc/aviation/comsystem.cpp index f93b01dc3..ac825f1f7 100644 --- a/src/blackmisc/aviation/comsystem.cpp +++ b/src/blackmisc/aviation/comsystem.cpp @@ -29,25 +29,15 @@ namespace BlackMisc qDBusRegisterMetaType(); } - bool CComSystem::validValues() const - { - if (this->isDefaultValue()) return true; // special case - return - (CComSystem::isValidCivilAviationFrequency(this->getFrequencyActive()) || - CComSystem::isValidMilitaryFrequency(this->getFrequencyActive())) && - (CComSystem::isValidCivilAviationFrequency(this->getFrequencyStandby()) || - CComSystem::isValidMilitaryFrequency(this->getFrequencyStandby())); - } - void CComSystem::setFrequencyActiveMHz(double frequencyMHz) { - CFrequency f(frequencyMHz, CFrequencyUnit::MHz()); + const CFrequency f(frequencyMHz, CFrequencyUnit::MHz()); this->setFrequencyActive(f); } void CComSystem::setFrequencyStandbyMHz(double frequencyMHz) { - CFrequency f(frequencyMHz, CFrequencyUnit::MHz()); + const CFrequency f(frequencyMHz, CFrequencyUnit::MHz()); this->setFrequencyStandby(f); } @@ -96,7 +86,7 @@ namespace BlackMisc CComSystem CComSystem::getCom1System(const CFrequency &activeFrequency, const CFrequency &standbyFrequency) { - return CComSystem(CModulator::NameCom1(), activeFrequency, standbyFrequency == CModulator::FrequencyNotSet() ? activeFrequency : standbyFrequency); + return CComSystem(CModulator::NameCom1(), activeFrequency, standbyFrequency.isNull() ? activeFrequency : standbyFrequency); } CComSystem CComSystem::getCom2System(double activeFrequencyMHz, double standbyFrequencyMHz) @@ -106,7 +96,7 @@ namespace BlackMisc CComSystem CComSystem::getCom2System(const CFrequency &activeFrequency, const CFrequency &standbyFrequency) { - return CComSystem(CModulator::NameCom2(), activeFrequency, standbyFrequency == CModulator::FrequencyNotSet() ? activeFrequency : standbyFrequency); + return CComSystem(CModulator::NameCom2(), activeFrequency, standbyFrequency.isNull() ? activeFrequency : standbyFrequency); } bool CComSystem::isValidCivilAviationFrequency(const CFrequency &f) diff --git a/src/blackmisc/aviation/comsystem.h b/src/blackmisc/aviation/comsystem.h index 014160c4a..e82f9ab1c 100644 --- a/src/blackmisc/aviation/comsystem.h +++ b/src/blackmisc/aviation/comsystem.h @@ -67,25 +67,25 @@ namespace BlackMisc CComSystem() {} //! Constructor - CComSystem(const QString &name, const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency = CModulator::FrequencyNotSet()): - CModulator(name, activeFrequency, standbyFrequency == CModulator::FrequencyNotSet() ? activeFrequency : standbyFrequency) + CComSystem(const QString &name, const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency = { 0, BlackMisc::PhysicalQuantities::CFrequencyUnit::nullUnit() }): + CModulator(name, activeFrequency, standbyFrequency.isNull() ? activeFrequency : standbyFrequency) { } //! Set active frequency //! \remarks will be rounded to channel spacing - virtual void setFrequencyActiveMHz(double frequencyMHz) override; + void setFrequencyActiveMHz(double frequencyMHz); //! Set standby frequency //! \remarks will be rounded to channel spacing - virtual void setFrequencyStandbyMHz(double frequencyMHz) override; + void setFrequencyStandbyMHz(double frequencyMHz); //! Set active frequency //! \remarks will be rounded to channel spacing - virtual void setFrequencyActive(const BlackMisc::PhysicalQuantities::CFrequency &frequency) override; + void setFrequencyActive(const BlackMisc::PhysicalQuantities::CFrequency &frequency); //! Set active frequency //! \remarks will be rounded to channel spacing - virtual void setFrequencyStandby(const BlackMisc::PhysicalQuantities::CFrequency &frequency) override; + void setFrequencyStandby(const BlackMisc::PhysicalQuantities::CFrequency &frequency); //! Is active frequency within 8.3383kHz channel? bool isActiveFrequencyWithin8_33kHzChannel(const BlackMisc::PhysicalQuantities::CFrequency &comFrequency) const; @@ -103,13 +103,15 @@ namespace BlackMisc static CComSystem getCom1System(double activeFrequencyMHz, double standbyFrequencyMHz = -1); //! COM1 unit - static CComSystem getCom1System(const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency = CModulator::FrequencyNotSet()); + static CComSystem getCom1System(const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, + const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency = { 0, BlackMisc::PhysicalQuantities::CFrequencyUnit::nullUnit() }); //! COM2 unit static CComSystem getCom2System(double activeFrequencyMHz, double standbyFrequencyMHz = -1); //! COM2 unit - static CComSystem getCom2System(const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency = CModulator::FrequencyNotSet()); + static CComSystem getCom2System(const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, + const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency = { 0, BlackMisc::PhysicalQuantities::CFrequencyUnit::nullUnit() }); //! Valid civil aviation frequency? static bool isValidCivilAviationFrequency(const BlackMisc::PhysicalQuantities::CFrequency &f); @@ -122,18 +124,17 @@ namespace BlackMisc //! Round to channel spacing, set MHz as unit //! \see ChannelSpacing - static void roundToChannelSpacing(BlackMisc::PhysicalQuantities::CFrequency &frequency, ChannelSpacing channelSpacing); + static void roundToChannelSpacing(BlackMisc::PhysicalQuantities::CFrequency &frequency, + ChannelSpacing channelSpacing); //! Is compareFrequency within channel spacing of setFrequency - static bool isWithinChannelSpacing(const BlackMisc::PhysicalQuantities::CFrequency &setFrequency, const BlackMisc::PhysicalQuantities::CFrequency &compareFrequency, ChannelSpacing channelSpacing); + static bool isWithinChannelSpacing(const BlackMisc::PhysicalQuantities::CFrequency &setFrequency, + const BlackMisc::PhysicalQuantities::CFrequency &compareFrequency, + ChannelSpacing channelSpacing); //! \copydoc BlackMisc::CValueObject::registerMetadata static void registerMetadata(); - protected: - //! \copydoc CModulator::validValues - virtual bool validValues() const override; - private: ChannelSpacing m_channelSpacing = ChannelSpacing25KHz; //!< channel spacing diff --git a/src/blackmisc/aviation/modulator.cpp b/src/blackmisc/aviation/modulator.cpp index ad70e1a68..9b67e8c30 100644 --- a/src/blackmisc/aviation/modulator.cpp +++ b/src/blackmisc/aviation/modulator.cpp @@ -17,6 +17,7 @@ #include "blackmisc/variant.h" #include "blackmisc/comparefunctions.h" #include +#include using BlackMisc::PhysicalQuantities::CFrequency; using BlackMisc::PhysicalQuantities::CFrequencyUnit; @@ -25,16 +26,10 @@ namespace BlackMisc { namespace Aviation { - template - bool CModulator::isDefaultValue() const - { - return (this->m_frequencyActive == FrequencyNotSet()); - } - template void CModulator::toggleActiveStandby() { - CFrequency a = this->m_frequencyActive; + const CFrequency a = this->m_frequencyActive; this->m_frequencyActive = this->m_frequencyStandby; this->m_frequencyStandby = a; } @@ -180,15 +175,17 @@ namespace BlackMisc } template - CModulator::CModulator() : - m_name("default") {} + CModulator::CModulator() : m_name("default") + { + static_assert(!std::is_polymorphic::value, "Must not use virtual functions for value classes"); + } template CModulator::CModulator(const QString &name, const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency) : - m_name(name), m_frequencyActive(activeFrequency), m_frequencyStandby(standbyFrequency) {} - - template - CModulator::~CModulator() {} + m_name(name), m_frequencyActive(activeFrequency), m_frequencyStandby(standbyFrequency) + { + static_assert(!std::is_polymorphic::value, "Must not use virtual functions for value classes"); + } template QString CModulator::convertToQString(bool i18n) const @@ -211,21 +208,6 @@ namespace BlackMisc this->m_frequencyStandby = BlackMisc::PhysicalQuantities::CFrequency(frequencyKHz, BlackMisc::PhysicalQuantities::CFrequencyUnit::kHz()); } - - template - void CModulator::setFrequencyActiveMHz(double frequencyMHz) - { - frequencyMHz = Math::CMathUtils::round(frequencyMHz, 3); - this->m_frequencyActive = BlackMisc::PhysicalQuantities::CFrequency(frequencyMHz, BlackMisc::PhysicalQuantities::CFrequencyUnit::MHz()); - } - - template - void CModulator::setFrequencyStandbyMHz(double frequencyMHz) - { - frequencyMHz = Math::CMathUtils::round(frequencyMHz, 3); - this->m_frequencyStandby = BlackMisc::PhysicalQuantities::CFrequency(frequencyMHz, BlackMisc::PhysicalQuantities::CFrequencyUnit::MHz()); - } - template const QString &CModulator::NameCom1() { @@ -282,13 +264,6 @@ namespace BlackMisc return n; } - template - const BlackMisc::PhysicalQuantities::CFrequency &CModulator::FrequencyNotSet() - { - static BlackMisc::PhysicalQuantities::CFrequency f; - return f; - } - // see here for the reason of the forward instantiations // https://isocpp.org/wiki/faq/templates#separate-template-fn-defn-from-decl //! \cond PRIVATE diff --git a/src/blackmisc/aviation/modulator.h b/src/blackmisc/aviation/modulator.h index 1e59c7e50..bda662172 100644 --- a/src/blackmisc/aviation/modulator.h +++ b/src/blackmisc/aviation/modulator.h @@ -33,9 +33,6 @@ namespace BlackMisc IndexEnabled }; - //! Default value? - virtual bool isDefaultValue() const; - //! Toggle active and standby frequencies void toggleActiveStandby(); @@ -45,12 +42,6 @@ namespace BlackMisc //! Standby frequency BlackMisc::PhysicalQuantities::CFrequency getFrequencyStandby() const; - //! Set active frequency - virtual void setFrequencyActive(const BlackMisc::PhysicalQuantities::CFrequency &frequency); - - //! Set standby frequency - virtual void setFrequencyStandby(const BlackMisc::PhysicalQuantities::CFrequency &frequency); - //! Output volume 0..100 int getVolumeOutput() const; @@ -72,9 +63,6 @@ namespace BlackMisc //! Enabled? void setEnabled(bool enable); - //! Are set values valid? - virtual bool validValues() const { return true; } - //! \copydoc BlackMisc::Mixin::Index::propertyByIndex CVariant propertyByIndex(const BlackMisc::CPropertyIndex &index) const; @@ -87,9 +75,6 @@ namespace BlackMisc //! \copydoc BlackMisc::Mixin::String::toQString QString convertToQString(bool i18n = false) const; - //! Destructor - virtual ~CModulator(); - protected: //! Default constructor CModulator(); @@ -97,18 +82,18 @@ namespace BlackMisc //! Constructor CModulator(const QString &name, const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency); + //! Set active frequency + void setFrequencyActive(const BlackMisc::PhysicalQuantities::CFrequency &frequency); + + //! Set standby frequency + void setFrequencyStandby(const BlackMisc::PhysicalQuantities::CFrequency &frequency); + //! Set active frequency void setFrequencyActiveKHz(double frequencyKHz); //! Set standby frequency void setFrequencyStandbyKHz(double frequencyKHz); - //! Set active frequency - virtual void setFrequencyActiveMHz(double frequencyMHz); - - //! Set standby frequency - virtual void setFrequencyStandbyMHz(double frequencyMHz); - //! COM1 static const QString &NameCom1(); @@ -133,9 +118,6 @@ namespace BlackMisc //! ADF2 static const QString &NameAdf2(); - //! Frequency not set - static const BlackMisc::PhysicalQuantities::CFrequency &FrequencyNotSet(); - private: QString m_name; //!< name of the unit BlackMisc::PhysicalQuantities::CFrequency m_frequencyActive; //!< active frequency diff --git a/src/blackmisc/aviation/navsystem.h b/src/blackmisc/aviation/navsystem.h index 32a52e543..24e38fdbc 100644 --- a/src/blackmisc/aviation/navsystem.h +++ b/src/blackmisc/aviation/navsystem.h @@ -19,7 +19,6 @@ namespace BlackMisc { namespace Aviation { - //! NAV system (radio navigation) class BLACKMISC_EXPORT CNavSystem : public CModulator, @@ -43,31 +42,39 @@ namespace BlackMisc { } //! Set active frequency - void setFrequencyActiveMHz(double frequencyMHz) override + void setFrequencyActiveMHz(double frequencyMHz) { - this->CModulator::setFrequencyActiveMHz(frequencyMHz); + const BlackMisc::PhysicalQuantities::CFrequency f(Math::CMathUtils::round(frequencyMHz, 3), BlackMisc::PhysicalQuantities::CFrequencyUnit::MHz()); + this->setFrequencyActive(f); } //! Set standby frequency - void setFrequencyStandbyMHz(double frequencyMHz) override + void setFrequencyStandbyMHz(double frequencyMHz) { - this->CModulator::setFrequencyStandbyMHz(frequencyMHz); + const BlackMisc::PhysicalQuantities::CFrequency f(Math::CMathUtils::round(frequencyMHz, 3), BlackMisc::PhysicalQuantities::CFrequencyUnit::MHz()); + this->setFrequencyStandby(f); } //! Valid civil aviation frequency? static bool isValidCivilNavigationFrequency(const BlackMisc::PhysicalQuantities::CFrequency &f) { - double fr = f.valueRounded(BlackMisc::PhysicalQuantities::CFrequencyUnit::MHz(), 3); + const double fr = f.valueRounded(BlackMisc::PhysicalQuantities::CFrequencyUnit::MHz(), 3); return fr >= 108.0 && fr <= 117.95; } //! Valid military aviation frequency? static bool isValidMilitaryNavigationFrequency(const BlackMisc::PhysicalQuantities::CFrequency &f) { - double fr = f.valueRounded(BlackMisc::PhysicalQuantities::CFrequencyUnit::MHz(), 3); + const double fr = f.valueRounded(BlackMisc::PhysicalQuantities::CFrequencyUnit::MHz(), 3); return fr >= 960.0 && fr <= 1215.0; // valid TACAN frequency } + //! Valid aviation frequency (military/civil) + static bool isValidNavigationFrequency(const BlackMisc::PhysicalQuantities::CFrequency &f) + { + return isValidCivilNavigationFrequency(f) || isValidMilitaryNavigationFrequency(f); + } + //! NAV1 unit static CNavSystem getNav1System(double activeFrequencyMHz, double standbyFrequencyMHz = -1) { @@ -75,9 +82,10 @@ namespace BlackMisc } //! NAV1 unit - static CNavSystem getNav1System(const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency = CModulator::FrequencyNotSet()) + static CNavSystem getNav1System(const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, + const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency = { 0, BlackMisc::PhysicalQuantities::CFrequencyUnit::nullUnit() }) { - return CNavSystem(CModulator::NameNav1(), activeFrequency, standbyFrequency == CModulator::FrequencyNotSet() ? activeFrequency : standbyFrequency); + return CNavSystem(CModulator::NameNav1(), activeFrequency, standbyFrequency.isNull() ? activeFrequency : standbyFrequency); } //! NAV2 unit @@ -87,22 +95,10 @@ namespace BlackMisc } //! NAV2 unit - static CNavSystem getNav2System(const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency = CModulator::FrequencyNotSet()) + static CNavSystem getNav2System(const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, + const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency = { 0, BlackMisc::PhysicalQuantities::CFrequencyUnit::nullUnit() }) { - return CNavSystem(CModulator::NameNav2(), activeFrequency, standbyFrequency == CModulator::FrequencyNotSet() ? activeFrequency : standbyFrequency); - } - - protected: - //! \copydoc CModulator::validValues - virtual bool validValues() const override - { - if (this->isDefaultValue()) return true; // special case - bool v = - (this->isValidCivilNavigationFrequency(this->getFrequencyActive()) || - this->isValidMilitaryNavigationFrequency(this->getFrequencyActive())) && - (this->isValidCivilNavigationFrequency(this->getFrequencyStandby()) || - this->isValidMilitaryNavigationFrequency(this->getFrequencyStandby())); - return v; + return CNavSystem(CModulator::NameNav2(), activeFrequency, standbyFrequency.isNull() ? activeFrequency : standbyFrequency); } private: @@ -112,7 +108,6 @@ namespace BlackMisc //! Easy access to derived class (CRTP template parameter) CNavSystem *derived() { return static_cast(this); } }; - } // namespace } // namespace diff --git a/src/blackmisc/aviation/transponder.cpp b/src/blackmisc/aviation/transponder.cpp index f7d2e5bac..3d24290fb 100644 --- a/src/blackmisc/aviation/transponder.cpp +++ b/src/blackmisc/aviation/transponder.cpp @@ -54,7 +54,6 @@ namespace BlackMisc bool CTransponder::validValues() const { - if (this->isDefaultValue()) return true; // special case return CTransponder::isValidTransponderCode(this->m_transponderCode); }