Cleaned up Entry PilotEntry, refactored NewPilotLog setups, commented [G] all around

src/experimental/DataBase.cpp

@@ -11,7 +11,8 @@ DataBase* DataBase::instance = nullptr;
 
 DataBase* DataBase::getInstance()
 {
-    if(!instance) instance = new DataBase();
+    if(!instance)
+        instance = new DataBase();
     return instance;
 }
 
@@ -19,19 +20,20 @@ bool DataBase::connect()
 {
     const QString driver("QSQLITE");
 
-    if (!QSqlDatabase::isDriverAvailable(driver)) return false;
+    if (!QSqlDatabase::isDriverAvailable(driver))
+        return false;
 
     QDir directory("data");
     QString databaseLocation = directory.filePath("logbook.db");
     QSqlDatabase db = QSqlDatabase::addDatabase(driver);
     db.setDatabaseName(databaseLocation);
 
-    if (!db.open()) return false;
+    if (!db.open())
+        return false;
 
     DEB("Database connection established.");
     // Enable foreign key restrictions
     QSqlQuery query("PRAGMA foreign_keys = ON;");
-    // Retreive database layout and store in member variables
     tableNames = db.tables();
 
     QStringList columnNames;
@@ -76,13 +78,13 @@ bool DataBase::remove(Entry entry)
         return false;
     }
 
-    QString statement = "DELETE FROM " + entry.position.tableName +
-            " WHERE _rowid_=" + QString::number(entry.position.rowId);
+    QString statement = "DELETE FROM " + entry.getPosition().tableName +
+            " WHERE _rowid_=" + QString::number(entry.getPosition().rowId);
     QSqlQuery q(statement);
-    //check result.
+
     if (q.lastError().type() == QSqlError::NoError)
     {
-        DEB("Entry " << entry.position.tableName << entry.position.rowId << " removed.");
+        DEB("Entry " << entry.getPosition().tableName << entry.getPosition().rowId << " removed.");
         return true;
     } else {
         DEB("Unable to delete.");
@@ -94,18 +96,18 @@ bool DataBase::remove(Entry entry)
 
 bool DataBase::exists(Entry entry)
 {
-    if(entry.position == DEFAULT_PILOT_POSITION)
+    if(entry.getPosition() == DEFAULT_PILOT_POSITION)
         return false;
 
     //Check database for row id
-    QString statement = "SELECT COUNT(*) FROM " + entry.position.tableName +
-            " WHERE _rowid_=" + QString::number(entry.position.rowId);
+    QString statement = "SELECT COUNT(*) FROM " + entry.getPosition().tableName +
+            " WHERE _rowid_=" + QString::number(entry.getPosition().rowId);
     //this returns either 1 or 0 since row ids are unique
     QSqlQuery q(statement);
     q.next();
     int rowId = q.value(0).toInt();
     if (rowId) {
-        DEB("Entry " << entry.position << " exists.");
+        DEB("Entry " << entry.getPosition() << " exists.");
         return true;
     } else {
         DEB("Entry does not exist.");
@@ -117,7 +119,7 @@ bool DataBase::exists(Entry entry)
 bool DataBase::update(Entry updated_entry)
 {
     auto data = updated_entry.getData();
-    QString statement = "UPDATE " + updated_entry.position.tableName + " SET ";
+    QString statement = "UPDATE " + updated_entry.getPosition().tableName + " SET ";
     for (auto i = data.constBegin(); i != data.constEnd(); ++i) {
         if (i.key() != QString()) {
             statement += i.key() + QLatin1String("='") + i.value() + QLatin1String("', ");
@@ -126,23 +128,21 @@ bool DataBase::update(Entry updated_entry)
         }
     }
     statement.chop(2); // Remove last comma
-    statement.append(QLatin1String(" WHERE _rowid_=") + QString::number(updated_entry.position.rowId));
+    statement.append(QLatin1String(" WHERE _rowid_=") + QString::number(updated_entry.getPosition().rowId));
 
     DEB("UPDATE QUERY: " << statement);
     QSqlQuery q(statement);
-    //check result.
+
     if (q.lastError().type() == QSqlError::NoError)
     {
         DEB("Entry successfully committed.");
         emit commitSuccessful();
-        /// [F] emit DB(), &DataBase::commitSuccessful, NewPilotDialog, &NewPilotDialog::onCommitSuccessful);
         return true;
     } else {
         DEB("Unable to commit.");
         DEB("Query: " << statement);
         DEB("Query Error: " << q.lastError().text());
         emit commitUnsuccessful(q.lastError().text(), statement);
-        /// [F] emit DB(), &DataBase::commitSuccessful, NewPilotDialog, &NewPilotDialog::onCommitSuccessful);
         return false;
     }
 }
@@ -151,7 +151,7 @@ bool DataBase::insert(Entry newEntry)
 {
     auto data = newEntry.getData();
     DEB("Inserting...");
-    QString statement = "INSERT INTO " + newEntry.position.tableName + QLatin1String(" (");
+    QString statement = "INSERT INTO " + newEntry.getPosition().tableName + QLatin1String(" (");
     QMap<QString, QString>::iterator i;
     for (i = data.begin(); i != data.end(); ++i) {
         statement += i.key() + QLatin1String(", ");
@@ -170,14 +170,12 @@ bool DataBase::insert(Entry newEntry)
     {
         DEB("Entry successfully committed.");
         emit commitSuccessful();
-        /// [F] emit DB(), &DataBase::commitSuccessful, NewPilotDialog, &NewPilotDialog::onCommitSuccessful);
         return true;
     } else {
         DEB("Unable to commit.");
         DEB("Query: " << statement);
         DEB("Query Error: " << q.lastError().text());
         emit commitUnsuccessful(q.lastError().text(), statement);
-        /// [F] emit DB(), &DataBase::commitSuccessful, NewPilotDialog, &NewPilotDialog::onCommitSuccessful);
         return false;
     }
 
@@ -230,6 +228,7 @@ TableData DataBase::getEntryData(DataPosition dataPosition)
 }
 
 /// does the same as geteEntryData, but slower
+/// [G]: so delete?
 /*TableData DataBase::getEntryDataQsqlTableModel(DataPosition dataPosition)
 {
     // check table exists
@@ -264,14 +263,14 @@ TableData DataBase::getEntryData(DataPosition dataPosition)
 Entry DataBase::getEntry(DataPosition dataPosition)
 {
     Entry entry(dataPosition);
-    entry.setData(DataBase::getEntryData(dataPosition));
+    entry.setData(getEntryData(dataPosition));
     return entry;
 }
 
 PilotEntry DataBase::getPilotEntry(RowId rowId)
 {
     PilotEntry pilotEntry(rowId);
-    pilotEntry.setData(DataBase::getEntryData(pilotEntry.position));
+    pilotEntry.setData(getEntryData(pilotEntry.getPosition()));
     return pilotEntry;
 }
 

src/experimental/DataBase.h

@@ -45,7 +45,8 @@ public:
      * \brief Can be used to access the database connection.
      * \return The QSqlDatabase object pertaining to the connection.
      */
-    static QSqlDatabase database();
+    static
+    QSqlDatabase database();
 
     /*!
      * \brief Checks if an entry exists in the database, based on position data
@@ -92,10 +93,14 @@ public:
      * with only the RowId required as input.
      */
     PilotEntry getPilotEntry(RowId);
-    ///[F] the same can easily be implemented for tails/flights
+    // [G] TODO: Ensure PilotDialog works great and slowly move to
+    // other dialogs
 signals:
     void commitSuccessful();
 
+    // [G] small nitpick but i believe we should return the error in its pure SqlError form.
+    // its better for the interested object to do get any relevant data from the error itself.
+    // The database doesnt know what part of the error is "interesting", just that it happened.
     void commitUnsuccessful(const QString &sqlError, const QString &sqlStatement);
 
 };

src/experimental/Decl.h

@@ -37,7 +37,6 @@ struct DataPosition : QPair<TableName, RowId> {
     DataPosition(const DataPosition& other) = default;
     DataPosition& operator=(const DataPosition& other) = default;
 };
-
 auto const DEFAULT_PILOT_POSITION = DataPosition("pilots", 0);
 
 }

src/experimental/Entry.cpp

@@ -6,24 +6,39 @@ Entry::Entry(DataPosition position_)
     : position(position_)
 {}
 
+Entry::Entry(TableData table_data)
+    : tableData(table_data)
+{}
+
+Entry::Entry(DataPosition position_, TableData table_data)
+    : position(position_), tableData(table_data)
+{}
+
 void Entry::setData(TableData table_data)
 {
     tableData = table_data;
 }
 
+const DataPosition& Entry::getPosition()
+{
+    return position;
+}
+
 const TableData& Entry::getData()
 {
     return tableData;
 }
 
+PilotEntry::PilotEntry()
+    : Entry::Entry(DEFAULT_PILOT_POSITION)
+{}
+
 PilotEntry::PilotEntry(int row_id)
     : Entry::Entry(DataPosition("pilots", row_id))
 {}
 
-PilotEntry::PilotEntry(TableData fromNewPilotDialog)
-    : Entry::Entry(DEFAULT_PILOT_POSITION)
-{
-    setData(fromNewPilotDialog);
-}
+PilotEntry::PilotEntry(TableData table_data)
+    : Entry::Entry(DEFAULT_PILOT_POSITION, table_data)
+{}
 
 }  // namespace experimental

src/experimental/Entry.h

@@ -18,27 +18,32 @@ namespace experimental {
  *  and data for new and existing entries in the database to operate on.
  */
 class Entry {
-public:
-    DataPosition position;
 protected:
+    DataPosition position;
     TableData tableData;
 public:
-    Entry() = default;
+    Entry() = delete; // Demand specificity from default constructor
     Entry(const Entry&) = default;
     Entry& operator=(const Entry&) = default;
     Entry(DataPosition position_);
+    Entry(TableData table_data);
+    Entry(DataPosition position_, TableData table_data);
+
     void setData(TableData table_data);
+    void setPosition(DataPosition position_);
+
+    const DataPosition& getPosition();
     const TableData& getData();
 
 };
 
-class PilotEntry : public Entry {
+struct PilotEntry : public Entry {
 public:
-    PilotEntry() = default;
+    PilotEntry();
     PilotEntry(const PilotEntry& pe) = default;
     PilotEntry& operator=(const PilotEntry& pe) = default;
     PilotEntry(int row_id);
-    PilotEntry(TableData newPilotData);
+    PilotEntry(TableData table_data);
 };
 
 }

src/gui/dialogues/newpilotdialog.cpp

@@ -35,34 +35,45 @@
  * Mathias d'Arras
  * Martin Luther King, Jr.
  */
-static const auto NAME_RX = QLatin1String("(\\p{L}+(\\s|'|\\-)?\\s?(\\p{L}+)?\\s?)");
-static const auto FIRSTNAME_VALID = QPair<QString, QRegularExpression> {
+static
+const auto NAME_RX = QLatin1String("(\\p{L}+(\\s|'|\\-)?\\s?(\\p{L}+)?\\s?)");
+static
+const auto FIRSTNAME_VALID = QPair<QString, QRegularExpression> {
     "picfirstnameLineEdit", QRegularExpression(NAME_RX + NAME_RX + NAME_RX)};
-static const auto LASTNAME_VALID = QPair<QString, QRegularExpression> {
+static
+const auto LASTNAME_VALID = QPair<QString, QRegularExpression> {
     "piclastnameLineEdit", QRegularExpression(NAME_RX + NAME_RX + NAME_RX)};
-static const auto PHONE_VALID = QPair<QString, QRegularExpression> {
+static
+const auto PHONE_VALID = QPair<QString, QRegularExpression> {
     "phoneLineEdit", QRegularExpression("^[+]{0,1}[0-9\\-\\s]+")};
-static const auto EMAIL_VALID = QPair<QString, QRegularExpression> {
+static
+const auto EMAIL_VALID = QPair<QString, QRegularExpression> {
     "emailLineEdit", QRegularExpression("\\A[a-z0-9!#$%&'*+/=?^_‘{|}~-]+(?:\\.[a-z0-9!#$%&'*+/=?^_‘{|}~-]+)*@"
                                         "(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\z")};
-static const auto COMPANY_VALID = QPair<QString, QRegularExpression> {
+static
+const auto COMPANY_VALID = QPair<QString, QRegularExpression> {
     "companyLineEdit", QRegularExpression("\\w+(\\s|\\-)?(\\w+(\\s|\\-)?)?(\\w+(\\s|\\-)?)?")};
-static const auto EMPLOYEENR_VALID = QPair<QString, QRegularExpression> {
+static
+const auto EMPLOYEENR_VALID = QPair<QString, QRegularExpression> {
     "employeeidLineEdit", QRegularExpression("\\w+")};
 
+static
+const auto LINE_EDIT_VALIDATORS = QVector{
+        FIRSTNAME_VALID,
+        LASTNAME_VALID,
+        PHONE_VALID,
+        EMAIL_VALID,
+        COMPANY_VALID,
+        EMPLOYEENR_VALID
+};
 
-static const auto LINE_EDIT_VALIDATORS = QVector({FIRSTNAME_VALID, LASTNAME_VALID,
-                                           PHONE_VALID,     EMAIL_VALID,
-                                           COMPANY_VALID,     EMPLOYEENR_VALID});
 // For creating a new entry
 NewPilotDialog::NewPilotDialog(QWidget *parent) :
     QDialog(parent),
     ui(new Ui::NewPilot)
 {
     DEB("New NewPilotDialog\n");
-    ui->setupUi(this);
-    setupValidators();
-    setupCompleter();
+    setup();
 
     using namespace experimental;
     pilotEntry = PilotEntry();
@@ -73,13 +84,11 @@ NewPilotDialog::NewPilotDialog(int rowId, QWidget *parent) :
     QDialog(parent),
     ui(new Ui::NewPilot)
 {
-    ui->setupUi(this);
-    setupValidators();
-    setupCompleter();
+    setup();
 
     using namespace experimental;
     pilotEntry = DB()->getPilotEntry(rowId);
-    DEB("Pilot Entry position: " << pilotEntry.position);
+    DEB("Pilot Entry position: " << pilotEntry.getPosition());
     formFiller();
     ui->piclastnameLineEdit->setFocus();
 }
@@ -90,6 +99,31 @@ NewPilotDialog::~NewPilotDialog()
     delete ui;
 }
 
+// [G]: Mover the two setup functions in one. The debug explains what happens
+// and we avoid 2 function calls with 1 potentially inlined one.
+void NewPilotDialog::setup()
+{
+    ui->setupUi(this);
+
+    DEB("Setting up Validators...");
+    for(const auto& pair : LINE_EDIT_VALIDATORS){
+        auto line_edit = parent()->findChild<QLineEdit*>(pair.first);
+        if(line_edit != nullptr){
+            auto validator = new QRegularExpressionValidator(pair.second,line_edit);
+            line_edit->setValidator(validator);
+        }else{
+            DEB("Error: Line Edit not found: "<< pair.first << " - skipping.");
+        }
+    }
+
+    DEB("Setting up completer...");
+    auto companies = new CompletionList(CompleterTarget::companies);
+    auto completer = new QCompleter(companies->list, ui->companyLineEdit);
+    completer->setCompletionMode(QCompleter::InlineCompletion);
+    completer->setCaseSensitivity(Qt::CaseSensitive);
+    ui->companyLineEdit->setCompleter(completer);
+}
+
 void NewPilotDialog::on_buttonBox_accepted()
 {
     if (ui->piclastnameLineEdit->text().isEmpty() || ui->picfirstnameLineEdit->text().isEmpty()) {
@@ -113,32 +147,6 @@ void NewPilotDialog::onCommitUnsuccessful(const QString &sqlError, const QString
                + sqlError);
 }
 
-void NewPilotDialog::setupValidators()
-{
-    DEB("Setting up Validators...");
-    for(const auto& pair : LINE_EDIT_VALIDATORS){
-        auto line_edit = parent()->findChild<QLineEdit*>(pair.first);
-        if(line_edit != nullptr){
-            auto validator = new QRegularExpressionValidator(pair.second,line_edit);
-            line_edit->setValidator(validator);
-        }else{
-            DEB("Error: Line Edit not found: "<< pair.first << " - skipping.");
-        }
-    }
-}
-
-void NewPilotDialog::setupCompleter()
-{
-    DEB("Setting up completer...");
-
-    auto companies = new CompletionList(CompleterTarget::companies);
-    auto completer = new QCompleter(companies->list, ui->companyLineEdit);
-    completer->setCompletionMode(QCompleter::InlineCompletion);
-    completer->setCaseSensitivity(Qt::CaseSensitive);
-
-    ui->companyLineEdit->setCompleter(completer);
-}
-
 void NewPilotDialog::formFiller()
 {
     DEB("Filling Form...");
@@ -164,6 +172,8 @@ void NewPilotDialog::submitForm()
         newData.insert(key, value);
     }
 
+    // [G]: If this formating is Entry-Subclass specific
+    // shouldnt PilotEntry know what to do with the database-centric pilot name?
     QString displayName;
     displayName.append(ui->piclastnameLineEdit->text());
     displayName.append(QLatin1String(", "));
@@ -174,7 +184,7 @@ void NewPilotDialog::submitForm()
     using namespace experimental;
 
     pilotEntry.setData(newData);
-    DEB("Pilot entry position: " << pilotEntry.position);
+    DEB("Pilot entry position: " << pilotEntry.getPosition());
     DEB("Pilot entry data: " << pilotEntry.getData());
     DB()->commit(pilotEntry);
 }

src/gui/dialogues/newpilotdialog.h

@@ -54,9 +54,7 @@ private:
 
     experimental::PilotEntry pilotEntry;
 
-    void setupValidators();
-
-    void setupCompleter();
+    inline void setup();
 
     void formFiller();