c++ – Code-review programs for working with notes

Question:

Hello. I am writing a small note-taking program. Now the minimal functionality has been implemented: a note can be created, edited and deleted. For the GUI, I have Qt, the base is SQLite. In general, everything works and does what I want. But my programming experience tends to zero, so I suspect that there are many shortcomings that I cannot fix, because I do not know where to look. I would like you to point out any bugs / problems / shortcomings that catch your eye. Interested in everything from the logic of division into classes to possible memory leaks.

Now there are essentially 12 classes:

  • 4 windows: MattyNotesMainWindow , MattySettingsDialog , addNoteDialog , MattyMessageBox
  • class for working with DB DbManager
  • class for composing SQL queries QueryConstructor
  • the actual class-note MattyNote
  • a constructor class for visually displaying the MattyGroupBox note
  • the class that deals with sorting and displaying the NoteHolder notes
  • class to manage css MattyStyleSheetEditor
  • class for storing strings and symbols Constants
  • watch MattyClocks

I understand that hardly anyone will read all the code, so maybe you can look at the classes and their methods and say what is superfluous, or look at one class and point out the shortcomings in it.

For example, the functions of connecting to the database, editing an existing note and extracting all notes from the database:

bool DbManager::connect(const QString & path)
{
    MattyNotesDb = QSqlDatabase::addDatabase("QSQLITE");
    MattyNotesDb.setDatabaseName(path);

    if(QFile::exists(path))
    {
        if (!MattyNotesDb.open())
        {
            showIsNotOpenedError();
            MattyNotesDb.close();

            return false;
        }
        else
        {
            PathToDb = MattyNotesDb.databaseName();
        }

        return true;
    }
    else
    {
        return false;
    }
}

bool DbManager::editNote(MattyNote & Note, int NoteId)
{
    if (connected())
    {
        QueryConstructor Edit;
        Edit.setTableName(QStringLiteral("Notes"));

        Edit.addWhereFieldValue(QStringLiteral("NoteId"), QString::number(NoteId));

        QMap<QString, QString> NoteTemp;
        NoteTemp["NoteTitle"] = "\'" + Note.getTitle() + "\'";
        NoteTemp["NoteType"] = "\'" + Note.getType() + "\'";
        NoteTemp["NoteText"] = "\'" + Note.getText() + "\'";
        NoteTemp["EventTime"] = "\'" + Note.getEventTime() + "\'";
        NoteTemp["EventDate"] = "\'" + Note.getEventDate() + "\'";
        NoteTemp["CrTime"] = "\'" + Note.getCrTime() + "\'";
        NoteTemp["CrDate"] = "\'" + Note.getCrDate() + "\'";
        NoteTemp["TypeId"] = QString::number(Note.getTypeId());

        Edit.setWhatToSetFieldValue(NoteTemp); //

        QSqlQuery editNoteQuery;
        return editNoteQuery.exec(Edit.constructUpdateQuery());
    }
    else
    {
        return false;
    }
}

QVector<MattyNoteRow> DbManager::showNotes()
{
    if (connected())
    {
        QVector<MattyNoteRow> VectorOfNoteRows;

        QueryConstructor SelectAllNotes;
        SelectAllNotes.setTableName(QStringLiteral("Notes"));
        SelectAllNotes.setOrderByClause("NoteId", Descending);

        QSqlQuery getAllNotesQuery(MattyNotesDb);

        if( getAllNotesQuery.exec(SelectAllNotes.constructSelectQuery()))
        {
            while (getAllNotesQuery.next())
            {
                MattyNoteRow Row;

                Row.NoteId=getAllNotesQuery.value("NoteId").toInt();
                Row.NoteTitle=getAllNotesQuery.value("NoteTitle").toString();
                Row.NoteType=getAllNotesQuery.value("NoteType").toString();
                Row.NoteText=getAllNotesQuery.value("NoteText").toString();
                Row.EventTime=getAllNotesQuery.value("EventTime").toString();
                Row.EventDate=getAllNotesQuery.value("EventDate").toString();
                Row.CrTime=getAllNotesQuery.value("CrTime").toString();
                Row.CrDate=getAllNotesQuery.value("CrDate").toString();
                Row.TypeId=getAllNotesQuery.value("TypeId").toInt();

                VectorOfNoteRows.push_back(Row);
            }
        }
        else
        {
            QMessageBox::critical(NULL, QObject::tr("Error"), getAllNotesQuery.lastError().text());
        }

        return VectorOfNoteRows;
    }
    else
    {
        return QVector<MattyNoteRow>();
    }
}

Submitting notes to the form:

    void NoteHolder::publishNotes(QWidget* ParentWidget)
    {
        erasePublishedNotes(ParentWidget);

        getAllNotes();

        QVector<class MattyNote>::iterator NoteNumber;
        int i;
        for (NoteNumber = ListOfAllNotes.begin(), i=0; NoteNumber < ListOfAllNotes.end();NoteNumber++, i++)
        {

            MattyGroupBox* MyGroupBox = new MattyGroupBox(*NoteNumber, ParentWidget);

            ParentWidget->layout()->addWidget(MyGroupBox);
        }
    }

void NoteHolder::erasePublishedNotes(QWidget* ParentWidget)
{
    MattyGroupBox* MgbTemp;
    while ((MgbTemp = ParentWidget->findChild<MattyGroupBox*>()) != 0)
    {
        delete MgbTemp;
    }

    QGroupBox* GbTemp;
    while ((GbTemp = ParentWidget->findChild<QGroupBox*>()) != 0)
    {
        delete GbTemp;
    }
}

void NoteHolder::getAllNotes()
{
    TotalNoteCount = 0;

    if (!ListOfAllNotes.isEmpty())
        ListOfAllNotes.clear();

    QVector<struct MattyNoteRow> ListOfRows = DbManager::showNotes();

    for (int i = 0; i < ListOfRows.length();i++)
    {
        MattyNote TempNote(ListOfRows[i]);
        ListOfAllNotes.append(TempNote);
        TotalNoteCount++;
    }
}

All source code can be seen here: GitHub

Division into classes, their methods, calls, dependencies, etc. here: Doxygen

Answer:

Decide on the storage of include files

All included files must be in the *.h header files, while you have half in *.cpp . Bring the application to a single form, place all unnecessary connections in the headers.

Universal paths

Now you have most of the paths and settings hardwired into the application, this is very bad, because every time you have to recompile and the program loses its versatility. It is much easier for the user to use more general tools in the form of configuration files.

Config.h

class Config
{
    public:
        static QString GetValue(QString);
    private:
        Config();
};

Config.cpp

Config::Config() { }

QString Config::GetValue(QString key)
{
    QString configPath = QCoreApplication::applicationDirPath() + "/config.ini";
    QSettings settings(configPath,  QSettings::IniFormat);

    return settings.value(key).toString();
}

Now you can clean up a bunch of unnecessary DbManager from the DbManager , also read the documentation, a lot can be taken out of there. Now you do not need to transfer lines, they will be read from the config.ini file themselves, or the path will be set by default to the directory with the executable file. Also, you are not closing the database connection, so I think you can use a simple DbManager::MattyNotesDb.isOpen() in DbManager::connected() .

QString DbManager::StoredPathDB = "";
QSqlDatabase DbManager::MattyNotesDb;

QString DbManager::GetPathDB()
{
    if(DbManager::StoredPathDB.isNull() || DbManager::StoredPathDB.isEmpty())
    {
        QString cPath = Config::GetValue("PathToDb");
        if(cPath.isNull() || cPath.isEmpty())
        {
            cPath = QCoreApplication::applicationDirPath() + "/MattyNotes.sqlite";
        }

        DbManager::StoredPathDB = cPath;
    }

    return StoredPathDB;
}

bool DbManager::connected()
{
    return DbManager::MattyNotesDb.isOpen();
}

bool DbManager::connect()
{
    MattyNotesDb = QSqlDatabase::addDatabase("QSQLITE");
    MattyNotesDb.setDatabaseName(DbManager::GetPathDB());

    if (!MattyNotesDb.open())
    {
        showIsNotOpenedError();
        MattyNotesDb.close();

        return false;
    }

    return true;
}

Now you can drop the application onto a USB flash drive and it should work everywhere.

Single responsibility principle

Each class should be responsible for its own purpose, it should not carry several functions. Do not forget to move the formation of entities inside them, without smearing them over the code of other classes.

MattyNoteRow(QSqlQuery query)
{
        NoteId=query.value("NoteId").toInt();
        NoteTitle=query.value("NoteTitle").toString();
        NoteType=query.value("NoteType").toString();
        NoteText=query.value("NoteText").toString();
        EventTime=query.value("EventTime").toString();
        EventDate=query.value("EventDate").toString();
        CrTime=query.value("CrTime").toString();
        CrDate=query.value("CrDate").toString();
        TypeId=query.value("TypeId").toInt();
}

Now requesting all notes looks better:

while (getNotesQuery.next())
{
    VectorOfNotes.push_back(MattyNoteRow(getNotesQuery));
}

A little more about DB

In general, it's good to use one DbManager::MattyNotesDb for everything and static methods are not the best solution. Problems will begin when the application suddenly acquires multithreading or some other entity. Therefore, it is better not to use this practice. Do not be afraid to write inflexible methods in the model. designing a model is very different from designing an application. Dynamic formation is left as a last resort.

This is due to the fact that it is quite difficult to recreate the request and execute it on the database in this form or to catch errors. Also, several people often work on a project and explicit requests make communication much easier. Sometimes it is enough to look at the request and find the error or places where changes are required. Dynamic formation deprives you of this, and you will have to shovel all the code and recreate requests from the code. Therefore, they make hard requests more often, but they are more effective.

It is also worth creating a separate class for entities. This is how it should look in the end.

DbManager.cpp

QSqlDatabase DbManager::GetInstance()
{
    QSqlDatabase db = QSqlDatabase::addDatabase("QSQLITE");
    db.setDatabaseName(DbManager::GetPathDB());

    return db;
}

MattyNoteProvider.cpp

MattyNoteRow MattyNoteProvider::ShowNote(int id)
{
    QSqlDatabase db = DbManager::GetInstance();

    if (db.open())
    {
        QString sql = "SELECT * FROM Notes WHERE NoteID = :noteID ORDER BY ASC";

        QSqlQuery query;
        query.prepare(sql);
        query.bindValue(":noteID", id);
        query.exec();

        ...

        db.close();
        return row;
    }
}
Scroll to Top