Improvements to models and delegates

Trying to  find the  sweet spot  for completeness,  usefulness, safety
and,  performance  from  the  various  custom  item  models  and  item
delegates.

git-svn-id: svn+ssh://svn.code.sf.net/p/wsjt/wsjt/branches/wsjtx@5474 ab8295b8-cf94-4d9e-aec4-7959e3be5d79

Bands.cpp

@@ -7,11 +7,14 @@
 
 namespace
 {
-  // Local structure to hold a single ADIF band definition.
-
   // Table of ADIF band definitions as defined in the ADIF
   // specification.
-  Bands::ADIFBand constexpr ADIF_bands[] = {
+  struct ADIFBand
+  {
+    char const * const name_;
+    Radio::Frequency lower_bound_;
+    Radio::Frequency upper_bound_;
+  } constexpr ADIF_bands[] = {
     {"2190m",	136000u,  		137000u},
     {"630m",  472000u,  		479000u},
     {"560m",  501000u,  		504000u},
@@ -44,7 +47,7 @@ namespace
     {"1mm",   241000000000u,250000000000u},
   };
 
-  Bands::ADIFBand constexpr oob = {"OOB", 0, std::numeric_limits<Bands::Frequency>::max ()};
+  QString const oob_name {QObject::tr ("OOB")};
 
   int constexpr table_rows ()
   {
@@ -57,22 +60,23 @@ Bands::Bands (QObject * parent)
 {
 }
 
-auto Bands::find (Frequency f) const -> ADIFBand const *
+QString Bands::find (Frequency f) const
 {
+  QString result;
   auto const& end_iter = ADIF_bands + table_rows ();
   auto const& row_iter = std::find_if (ADIF_bands, end_iter, [f] (ADIFBand const& band) {
       return band.lower_bound_ <= f && f <= band.upper_bound_;
     });
   if (row_iter != end_iter)
     {
-      return row_iter;
+      result = row_iter->name_;
     }
-  return &oob;
+  return result;
 }
 
-auto Bands::out_of_band () const -> ADIFBand const *
+QString const& Bands::oob ()
 {
-  return &oob;
+  return oob_name;
 }
 
 int Bands::rowCount (QModelIndex const& parent) const
@@ -99,7 +103,7 @@ QVariant Bands::data (QModelIndex const& index, int role) const
       // Hijack root for OOB string.
       if (Qt::DisplayRole == role)
         {
-          item = oob.name_;
+          item = oob_name;
         }
     }
   else

Bands.hpp

@@ -15,9 +15,9 @@
 // Responsibilities
 //
 //  Provides  a  well  known  band  name mapped  to  lower  and  upper
-//  frequency  limits.  Also  provides   a  convenience  operation  to
+//  frequency  limits.   Also  provides  a  convenience  operation  to
-//  determine the band details for  any given frequency, the result of
+//  determine the  band name  for any given  frequency, the  result of
-//  which may be  invalid if the given frequency doesn't  lie within a
+//  which may  be null  if the  given frequency  doesn't lie  within a
 //  recognised band.
 //
 // Collaborations
@@ -32,20 +32,13 @@ class Bands final
 public:
   using Frequency = Radio::Frequency;
 
-  struct ADIFBand
-  {
-    char const * const name_;
-    Radio::Frequency lower_bound_;
-    Radio::Frequency upper_bound_;
-  };
-
   explicit Bands (QObject * parent = nullptr);
 
   //
   // Model API
   //
-  ADIFBand const * find (Frequency) const; // find band Frequency is in
+  QString find (Frequency) const; // find band Frequency is in
-  ADIFBand const * out_of_band () const;
+  static QString const& oob ();
 
   // Custom role for sorting.
   static int constexpr SortRole = Qt::UserRole;

CMakeLists.txt

@@ -187,7 +187,6 @@ set (wsjt_qt_CXXSRCS
   FrequencyList.cpp
   StationList.cpp
   FrequencyLineEdit.cpp
-  FrequencyItemDelegate.cpp
   CandidateKeyFilter.cpp
   ForeignKeyDelegate.cpp
   LiveFrequencyValidator.cpp

Configuration.cpp

@@ -163,9 +163,9 @@
 #include <QDebug>
 
 #include "qt_helpers.hpp"
+#include "MetaDataRegistry.hpp"
 #include "SettingsGroup.hpp"
 #include "FrequencyLineEdit.hpp"
-#include "FrequencyItemDelegate.hpp"
 #include "CandidateKeyFilter.hpp"
 #include "ForeignKeyDelegate.hpp"
 #include "TransceiverFactory.hpp"
@@ -884,9 +884,15 @@ Configuration::impl::impl (Configuration * self, QSettings * settings, QWidget *
 
   ui_->frequencies_table_view->setModel (&next_frequencies_);
   ui_->frequencies_table_view->sortByColumn (FrequencyList::frequency_column, Qt::AscendingOrder);
-  ui_->frequencies_table_view->setItemDelegateForColumn (FrequencyList::mode_column, new ForeignKeyDelegate {&modes_, 0, this});
   ui_->frequencies_table_view->setColumnHidden (FrequencyList::frequency_mhz_column, true);
 
+  // delegates
+  auto frequencies_item_delegate = new QStyledItemDelegate {this};
+  frequencies_item_delegate->setItemEditorFactory (item_editor_factory ());
+  ui_->frequencies_table_view->setItemDelegate (frequencies_item_delegate);
+  ui_->frequencies_table_view->setItemDelegateForColumn (FrequencyList::mode_column, new ForeignKeyDelegate {&modes_, 0, this});
+
+  // actions
   frequency_delete_action_ = new QAction {tr ("&Delete"), ui_->frequencies_table_view};
   ui_->frequencies_table_view->insertAction (nullptr, frequency_delete_action_);
   connect (frequency_delete_action_, &QAction::triggered, this, &Configuration::impl::delete_frequencies);
@@ -903,8 +909,14 @@ Configuration::impl::impl (Configuration * self, QSettings * settings, QWidget *
 
   ui_->stations_table_view->setModel (&next_stations_);
   ui_->stations_table_view->sortByColumn (StationList::band_column, Qt::AscendingOrder);
+
+  // delegates
+  auto stations_item_delegate = new QStyledItemDelegate {this};
+  stations_item_delegate->setItemEditorFactory (item_editor_factory ());
+  ui_->stations_table_view->setItemDelegate (stations_item_delegate);
   ui_->stations_table_view->setItemDelegateForColumn (StationList::band_column, new ForeignKeyDelegate {&bands_, &next_stations_, 0, StationList::band_column, this});
 
+  // actions
   station_delete_action_ = new QAction {tr ("&Delete"), ui_->stations_table_view};
   ui_->stations_table_view->insertAction (nullptr, station_delete_action_);
   connect (station_delete_action_, &QAction::triggered, this, &Configuration::impl::delete_stations);

FrequencyItemDelegate.cpp

@@ -1,22 +0,0 @@
-#include "FrequencyItemDelegate.hpp"
-
-#include "FrequencyLineEdit.hpp"
-
-QWidget * FrequencyItemDelegate::createEditor (QWidget * parent
-					       , QStyleOptionViewItem const& /* option */
-					       , QModelIndex const& /* index */) const
-{
-  auto editor = new FrequencyLineEdit {parent};
-  editor->setFrame (false);
-  return editor;
-}
-
-
-QWidget * FrequencyDeltaItemDelegate::createEditor (QWidget * parent
-					       , QStyleOptionViewItem const& /* option */
-					       , QModelIndex const& /* index */) const
-{
-  auto editor = new FrequencyDeltaLineEdit {parent};
-  editor->setFrame (false);
-  return editor;
-}

FrequencyItemDelegate.hpp

@@ -1,45 +0,0 @@
-#ifndef FREQUENCY_ITEM_DELEGATE_HPP_
-#define FREQUENCY_ITEM_DELEGATE_HPP_
-
-#include <QStyledItemDelegate>
-
-//
-// Class FrequencyItemDelegate
-//
-//	Item delegate for displaying and editing a Frequency item in a
-//	view that uses a FrequencyLineEdit as an item delegate for the
-//	edit role.
-//
-class FrequencyItemDelegate final
-  : public QStyledItemDelegate
-{
-public:
-  explicit FrequencyItemDelegate (QObject * parent = nullptr)
-    : QStyledItemDelegate {parent}
-  {
-  }
-
-  QWidget * createEditor (QWidget * parent, QStyleOptionViewItem const&, QModelIndex const&) const override;
-};
-
-
-//
-// Class FrequencyDeltaItemDelegate
-//
-//	Item delegate for displaying and editing a FrequencyDelta item
-//	in a view that uses a FrequencyDeltaLineEdit as an item
-//	delegate for the edit role.
-//
-class FrequencyDeltaItemDelegate final
-  : public QStyledItemDelegate
-{
-public:
-  explicit FrequencyDeltaItemDelegate (QObject * parent = nullptr)
-    : QStyledItemDelegate {parent}
-  {
-  }
-
-  QWidget * createEditor (QWidget * parent, QStyleOptionViewItem const&, QModelIndex const&) const override;
-};
-
-#endif

FrequencyList.cpp

@@ -167,10 +167,11 @@ auto FrequencyList::frequency_list () const -> FrequencyItems const&
   return m_->frequency_list_;
 }
 
-QModelIndex FrequencyList::best_working_frequency (Frequency f, Mode mode) const
+int FrequencyList::best_working_frequency (Frequency f) const
 {
+  int result {-1};
   auto const& target_band = m_->bands_->find (f);
-  if (target_band != m_->bands_->out_of_band ())
+  if (!target_band.isEmpty ())
     {
       // find a frequency in the same band that is allowed for the
       // target mode
@@ -178,16 +179,13 @@ QModelIndex FrequencyList::best_working_frequency (Frequency f, Mode mode) const
         {
           auto const& source_row = mapToSource (index (row, 0)).row ();
           auto const& band = m_->bands_->find (m_->frequency_list_[source_row].frequency_);
-          if (band->name_ == target_band->name_)
+          if (band == target_band)
             {
-              if (m_->frequency_list_[source_row].mode_ == mode)
+              return row;
-                {
-                  return index (row, 0);
-                }
             }
         }
     }
-  return QModelIndex {};
+  return result;
 }
 
 void FrequencyList::reset_to_defaults ()
@@ -360,7 +358,8 @@ QVariant FrequencyList::impl::data (QModelIndex const& index, int role) const
             case Qt::DisplayRole:
               {
                 auto const& band = bands_->find (frequency_item.frequency_);
-                item = Radio::pretty_frequency_MHz_string (frequency_item.frequency_) + " MHz (" + band->name_ + ')';
+                item = Radio::pretty_frequency_MHz_string (frequency_item.frequency_)
+                  + " MHz (" + (band.isEmpty () ? "OOB" : band) + ')';
               }
               break;
 
@@ -386,7 +385,8 @@ QVariant FrequencyList::impl::data (QModelIndex const& index, int role) const
             case Qt::DisplayRole:
               {
                 auto const& band = bands_->find (frequency_item.frequency_);
-                item = Radio::pretty_frequency_MHz_string (frequency_item.frequency_) + " MHz (" + band->name_ + ')';
+                item = Radio::pretty_frequency_MHz_string (frequency_item.frequency_)
+                  + " MHz (" + (band.isEmpty () ? "OOB" : band) + ')';
               }
               break;
 
@@ -527,3 +527,29 @@ QMimeData * FrequencyList::impl::mimeData (QModelIndexList const& items) const
   mime_data->setData (mime_type, encoded_data);
   return mime_data;
 }
+
+auto FrequencyList::const_iterator::operator * () -> Item const&
+{
+  return parent_->frequency_list ().at(parent_->mapToSource (parent_->index (row_, 0)).row ());
+}
+
+bool FrequencyList::const_iterator::operator != (const_iterator const& rhs) const
+{
+  return parent_ != rhs.parent_ || row_ != rhs.row_;
+}
+
+auto FrequencyList::const_iterator::operator ++ () -> const_iterator&
+{
+  ++row_;
+  return *this;
+}
+
+auto FrequencyList::begin () const -> FrequencyList::const_iterator
+{
+  return const_iterator (this, 0);
+}
+
+auto FrequencyList::end () const -> FrequencyList::const_iterator
+{
+  return const_iterator (this, rowCount ());
+}

FrequencyList.hpp

@@ -50,15 +50,44 @@ public:
 
   enum Column {mode_column, frequency_column, frequency_mhz_column};
 
+  // an iterator that meets the requirements of the C++ for range statement
+  class const_iterator
+  {
+  public:
+    const_iterator (FrequencyList const * parent, int row)
+      : parent_ {parent}
+      , row_ {row}
+    {
+    }
+
+    Item const& operator * ();
+    bool operator != (const_iterator const&) const;
+    const_iterator& operator ++ ();
+
+  private:
+    FrequencyList const * parent_;
+    int row_;
+  };
+
   explicit FrequencyList (Bands const *, QObject * parent = nullptr);
   ~FrequencyList ();
 
-  // Load and store contents
+  // Load and store underlying items
   FrequencyItems frequency_list (FrequencyItems);
   FrequencyItems const& frequency_list () const;
 
-  // Find nearest best working frequency given a frequency and mode
+  // Iterators for the sorted and filtered items
-  QModelIndex best_working_frequency (Frequency, Mode) const;
+  //
+  // Note that these iterators are on the final sorted and filtered
+  // rows, if you need to access the underlying unfiltered and
+  // unsorted frequencies then use the frequency_list() member to
+  // access the underlying list of rows
+  const_iterator begin () const;
+  const_iterator end () const;
+
+  // Find the row of the nearest best working frequency given a
+  // frequency and mode
+  int best_working_frequency (Frequency) const;
 
   // Set filter
   void filter (Mode);

LiveFrequencyValidator.cpp

@@ -42,17 +42,16 @@ auto LiveFrequencyValidator::validate (QString& input, int& pos) const -> State
 void LiveFrequencyValidator::fixup (QString& input) const
 {
   QRegExpValidator::fixup (input);
-  auto const& out_of_band = bands_->out_of_band ();
+  if (!bands_->oob ().startsWith (input))
-  if (!QString {out_of_band->name_}.startsWith (input))
     {
       if (input.contains ('m', Qt::CaseInsensitive))
         {
           input = input.toLower ();
 
           QVector<QVariant> frequencies;
-          Q_FOREACH (auto const& item, frequencies_->frequency_list ())
+          for (auto const& item : frequencies_->frequency_list ())
             {
-              if (bands_->find (item.frequency_)->name_ == input)
+              if (bands_->find (item.frequency_) == input)
                 {
                   frequencies << item.frequency_;
                 }
@@ -70,11 +69,11 @@ void LiveFrequencyValidator::fixup (QString& input) const
         {
           // frequency input
           auto f = Radio::frequency (input, 6);
-          input = bands_->find (f)->name_;
+          input = bands_->find (f);
           Q_EMIT valid (f);
         }
 
-      if (bands_->out_of_band ()->name_ == input)
+      if (bands_->oob () == input)
         {
           combo_box_->lineEdit ()->setStyleSheet ("QLineEdit {color: yellow; background-color : red;}");
         }

MetaDataRegistry.cpp

@@ -15,16 +15,19 @@
 
 #include "FrequencyLineEdit.hpp"
 
+QItemEditorFactory * item_editor_factory ()
+{
+  static QItemEditorFactory * our_item_editor_factory = new QItemEditorFactory;
+  return our_item_editor_factory;
+}
+
 void register_types ()
 {
-  // Default custom item view delegates
-  auto factory = new QItemEditorFactory;
-
   // Radio namespace
   auto frequency_type_id = qRegisterMetaType<Radio::Frequency> ("Frequency");
-  factory->registerEditor (frequency_type_id, new QStandardItemEditorCreator<FrequencyLineEdit> ());
+  item_editor_factory ()->registerEditor (frequency_type_id, new QStandardItemEditorCreator<FrequencyLineEdit> ());
   auto frequency_delta_type_id = qRegisterMetaType<Radio::FrequencyDelta> ("FrequencyDelta");
-  factory->registerEditor (frequency_delta_type_id, new QStandardItemEditorCreator<FrequencyDeltaLineEdit> ());
+  item_editor_factory ()->registerEditor (frequency_delta_type_id, new QStandardItemEditorCreator<FrequencyDeltaLineEdit> ());
 
   // Frequency list model
   qRegisterMetaType<FrequencyList::Item> ("Item");
@@ -67,6 +70,4 @@ void register_types ()
 
   // Waterfall palette
   qRegisterMetaTypeStreamOperators<WFPalette::Colours> ("Colours");
-
-  QItemEditorFactory::setDefaultFactory (factory);
 }

MetaDataRegistry.hpp

@@ -1,6 +1,9 @@
 #ifndef META_DATA_REGISTRY_HPP__
 #define META_DATA_REGISTRY_HPP__
 
+class QItemEditorFactory;
+
+QItemEditorFactory * item_editor_factory ();
 void register_types ();
 
 #endif

StationList.cpp

@@ -203,12 +203,12 @@ auto StationList::impl::offset (Frequency f) const -> FrequencyDelta
 {
   // Lookup band for frequency
   auto const& band = bands_->find (f);
-  if (band != bands_->out_of_band ())
+  if (!band.isEmpty ())
     {
       // Lookup station for band
       for (int i = 0; i < stations_.size (); ++i)
         {
-          if (stations_[i].band_name_ == band->name_)
+          if (stations_[i].band_name_ == band)
             {
               return stations_[i].offset_;
             }
@@ -514,10 +514,10 @@ bool StationList::impl::dropMimeData (QMimeData const * data, Qt::DropAction act
           auto const& band = bands_->find (frequency);
           if (stations_.cend () == std::find_if (stations_.cbegin ()
                                                  , stations_.cend ()
-                                                 , [&band] (Station const& s) {return s.band_name_ == band->name_;}))
+                                                 , [&band] (Station const& s) {return s.band_name_ == band;}))
             {
               // not found so add it
-              add (Station {band->name_, 0, QString {}});
+              add (Station {band, 0, QString {}});
             }
         }
       return true;

StationList.hpp

@@ -35,8 +35,7 @@ class Bands;
 //  Implements the  QAbstractTableModel interface for a  grid of bands
 //  with offset frequencies and antenna descriptions.
 //
-//  Uses the QAbstractItemModel interface of the bands model to lookup
+//  Uses the Bands model to lookup band information.
-//  band information.
 //
 class StationList final
   : public QSortFilterProxyModel

mainwindow.cpp

@@ -33,7 +33,6 @@
 #include "FrequencyList.hpp"
 #include "StationList.hpp"
 #include "LiveFrequencyValidator.hpp"
-#include "FrequencyItemDelegate.hpp"
 #include "MessageClient.hpp"
 
 #include "ui_mainwindow.h"
@@ -1148,7 +1147,7 @@ void MainWindow::qsy (Frequency f)
 void MainWindow::displayDialFrequency ()
 {
   // lookup band
-  auto const& band_name = m_config.bands ()->find (m_dialFreq)->name_;
+  auto const& band_name = m_config.bands ()->find (m_dialFreq);
   ui->bandComboBox->setCurrentText (band_name);
   m_wideGraph->setRxBand (band_name);
 
@@ -1156,11 +1155,8 @@ void MainWindow::displayDialFrequency ()
   // of on VHF and up)
   bool valid {false};
   quint64 min_offset {99999999};
-  auto const& frequencies = m_config.frequencies ();
+  for (auto const& item : *m_config.frequencies ())
-  for (int row = 0; row < frequencies->rowCount (); ++row)
     {
-      auto const& source_row = frequencies->mapToSource (frequencies->index (row, 0)).row ();
-      auto const& item = frequencies->frequency_list ()[source_row];
       // we need to do specific checks for above and below here to
       // ensure that we can use unsigned Radio::Frequency since we
       // potentially use the full 64-bit unsigned range.
@@ -1653,7 +1649,7 @@ void MainWindow::readFromStdout()                             //readFromStdout
             QString band;
             if (QDateTime::currentMSecsSinceEpoch() / 1000 - m_secBandChanged > 50)
               {
-                band = ' ' + QString {m_config.bands ()->find (m_dialFreq)->name_};
+                band = ' ' + m_config.bands ()->find (m_dialFreq);
               }
             ui->decodedTextBrowser->insertLineSpacer (band.rightJustified  (40, '-'));
             m_blankLine = false;
@@ -2228,8 +2224,16 @@ void MainWindow::startTx2()
     transmit (snr);
     signalMeter->setValue(0);
     if(m_mode.mid(0,4)=="WSPR" and !m_tune) {
+<<<<<<< variant A
       t = " Transmitting " + m_mode + " ----------------------- " +
         QString {m_config.bands ()->find (m_dialFreq)->name_};
+>>>>>>> variant B
+      t = " Transmiting " + m_mode + " ----------------------- " +
+        m_config.bands ()->find (m_dialFreq);
+####### Ancestor
+      t = " Transmiting " + m_mode + " ----------------------- " +
+        QString {m_config.bands ()->find (m_dialFreq)->name_};
+======= end
       ui->decodedTextBrowser->append(t.rightJustified (71, '-'));
 
       QFile f {m_dataDir.absoluteFilePath ("ALL_WSPR.TXT")};
@@ -2963,7 +2967,7 @@ void MainWindow::acceptQSO2(QDateTime const& QSO_date, QString const& call, QStr
                             , QString const& name)
 {
   QString date = m_dateTimeQSO.toString("yyyyMMdd");
-  m_logBook.addAsWorked (m_hisCall, m_config.bands ()->find (m_dialFreq)->name_, m_modeTx, date);
+  m_logBook.addAsWorked (m_hisCall, m_config.bands ()->find (m_dialFreq), m_modeTx, date);
 
   m_messageClient->qso_logged (QSO_date, call, grid, dial_freq, mode, rpt_sent, rpt_received, tx_power, comments, name);
 
@@ -3181,11 +3185,11 @@ void MainWindow::switch_mode (Mode mode)
 {
   auto f = m_dialFreq;
   m_config.frequencies ()->filter (mode);
-  auto const& index = m_config.frequencies ()->best_working_frequency (f, mode);
+  auto const& row = m_config.frequencies ()->best_working_frequency (f);
-  if (index.isValid ())
+  if (row >= 0)
     {
-      ui->bandComboBox->setCurrentIndex (index.row ());
+      ui->bandComboBox->setCurrentIndex (row);
-      on_bandComboBox_activated (index.row ());
+      on_bandComboBox_activated (row);
     }
 }
 
@@ -3332,16 +3336,15 @@ void MainWindow::on_bandComboBox_currentIndexChanged (int index)
 
   // Lookup band
   auto const& band  = m_config.bands ()->find (frequency);
-  auto const& out_of_band  = m_config.bands ()->out_of_band ();
+  if (!band.isEmpty ())
-  if (out_of_band != band)
     {
       ui->bandComboBox->lineEdit ()->setStyleSheet ({});
-      ui->bandComboBox->setCurrentText (band->name_);
+      ui->bandComboBox->setCurrentText (band);
     }
   else
     {
       ui->bandComboBox->lineEdit ()->setStyleSheet ("QLineEdit {color: yellow; background-color : red;}");
-      ui->bandComboBox->setCurrentText (out_of_band->name_);
+      ui->bandComboBox->setCurrentText (m_config.bands ()->oob ());
     }
   displayDialFrequency ();
 }
@@ -3357,7 +3360,7 @@ void MainWindow::on_bandComboBox_activated (int index)
     }
   m_bandEdited = true;
   band_changed (frequency);
-  m_wideGraph->setRxBand (m_config.bands ()->find (frequency)->name_);
+  m_wideGraph->setRxBand (m_config.bands ()->find (frequency));
 }
 
 void MainWindow::band_changed (Frequency f)
@@ -4164,7 +4167,7 @@ void MainWindow::p1ReadFromStdout()                        //p1readFromStdout
       if (m_config.insert_blank () && m_blankLine) {
         QString band;
         Frequency f=1000000.0*rxFields.at(3).toDouble()+0.5;
-        band = ' ' + QString {m_config.bands ()->find (f)->name_};
+        band = ' ' + m_config.bands ()->find (f);
         ui->decodedTextBrowser->append(band.rightJustified (71, '-'));
         m_blankLine = false;
       }