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

@@ -21,7 +21,6 @@ namespace BlackMisc
 {
     namespace Aviation
     {
-
         //! ADF system ("for NDBs")
         class BLACKMISC_EXPORT CAdfSystem :
             public CModulator<CAdfSystem>,
@@ -40,8 +39,9 @@ namespace BlackMisc
             CAdfSystem() = default;
 
             //! Constructor
-            CAdfSystem(const QString &name, const PhysicalQuantities::CFrequency &activeFrequency, const PhysicalQuantities::CFrequency &standbyFrequency = CModulator::FrequencyNotSet()):
+            CAdfSystem(const QString &name, const PhysicalQuantities::CFrequency &activeFrequency,
-                CModulator(name, activeFrequency, standbyFrequency == CModulator::FrequencyNotSet() ? activeFrequency : standbyFrequency)
+                       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);
+                return CAdfSystem(CModulator::NameCom2(), activeFrequency, standbyFrequency.isNull() ? 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());
             }
         };
-    }
+    } // ns
-}
+} // ns
 
 Q_DECLARE_METATYPE(BlackMisc::Aviation::CAdfSystem)
 

src/blackmisc/aviation/comsystem.cpp

@@ -29,25 +29,15 @@ namespace BlackMisc
             qDBusRegisterMetaType<ComUnit>();
         }
 
-        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)

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()):
+            CComSystem(const QString &name, const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency = { 0, BlackMisc::PhysicalQuantities::CFrequencyUnit::nullUnit() }):
-                CModulator(name, activeFrequency, standbyFrequency == CModulator::FrequencyNotSet() ? activeFrequency : standbyFrequency)
+                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
 

src/blackmisc/aviation/modulator.cpp

@@ -17,6 +17,7 @@
 #include "blackmisc/variant.h"
 #include "blackmisc/comparefunctions.h"
 #include <QtGlobal>
+#include <type_traits>
 
 using BlackMisc::PhysicalQuantities::CFrequency;
 using BlackMisc::PhysicalQuantities::CFrequencyUnit;
@@ -25,16 +26,10 @@ namespace BlackMisc
 {
     namespace Aviation
     {
-        template <class AVIO>
-        bool CModulator<AVIO>::isDefaultValue() const
-        {
-            return (this->m_frequencyActive == FrequencyNotSet());
-        }
-
         template <class AVIO>
         void CModulator<AVIO>::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 <class AVIO>
-        CModulator<AVIO>::CModulator() :
+        CModulator<AVIO>::CModulator() : m_name("default")
-            m_name("default") {}
+        {
+            static_assert(!std::is_polymorphic<AVIO>::value, "Must not use virtual functions for value classes");
+        }
 
         template <class AVIO>
         CModulator<AVIO>::CModulator(const QString &name, const BlackMisc::PhysicalQuantities::CFrequency &activeFrequency, const BlackMisc::PhysicalQuantities::CFrequency &standbyFrequency) :
-            m_name(name), m_frequencyActive(activeFrequency), m_frequencyStandby(standbyFrequency) {}
+            m_name(name), m_frequencyActive(activeFrequency), m_frequencyStandby(standbyFrequency)
-
+        {
-        template <class AVIO>
+            static_assert(!std::is_polymorphic<AVIO>::value, "Must not use virtual functions for value classes");
-        CModulator<AVIO>::~CModulator() {}
+        }
 
         template <class AVIO>
         QString CModulator<AVIO>::convertToQString(bool i18n) const
@@ -211,21 +208,6 @@ namespace BlackMisc
             this->m_frequencyStandby = BlackMisc::PhysicalQuantities::CFrequency(frequencyKHz, BlackMisc::PhysicalQuantities::CFrequencyUnit::kHz());
         }
 
-
-        template <class AVIO>
-        void CModulator<AVIO>::setFrequencyActiveMHz(double frequencyMHz)
-        {
-            frequencyMHz = Math::CMathUtils::round(frequencyMHz, 3);
-            this->m_frequencyActive = BlackMisc::PhysicalQuantities::CFrequency(frequencyMHz, BlackMisc::PhysicalQuantities::CFrequencyUnit::MHz());
-        }
-
-        template <class AVIO>
-        void CModulator<AVIO>::setFrequencyStandbyMHz(double frequencyMHz)
-        {
-            frequencyMHz = Math::CMathUtils::round(frequencyMHz, 3);
-            this->m_frequencyStandby = BlackMisc::PhysicalQuantities::CFrequency(frequencyMHz, BlackMisc::PhysicalQuantities::CFrequencyUnit::MHz());
-        }
-
         template <class AVIO>
         const QString &CModulator<AVIO>::NameCom1()
         {
@@ -282,13 +264,6 @@ namespace BlackMisc
             return n;
         }
 
-        template <class AVIO>
-        const BlackMisc::PhysicalQuantities::CFrequency &CModulator<AVIO>::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

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

src/blackmisc/aviation/navsystem.h

@@ -19,7 +19,6 @@ namespace BlackMisc
 {
     namespace Aviation
     {
-
         //! NAV system (radio navigation)
         class BLACKMISC_EXPORT CNavSystem :
             public CModulator<CNavSystem>,
@@ -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);
+                return CNavSystem(CModulator::NameNav2(), activeFrequency, standbyFrequency.isNull() ? 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;
             }
 
         private:
@@ -112,7 +108,6 @@ namespace BlackMisc
             //! Easy access to derived class (CRTP template parameter)
             CNavSystem *derived() { return static_cast<CNavSystem *>(this); }
         };
-
     } // namespace
 } // namespace
 

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);
         }