10
\$\begingroup\$

This basically manages a set of snippets, saving and loading it.

snippets.cpp

#include "Features/snippets.h"
#include <iostream>

Snippets::Snippets(QObject* parent)
    : QObject(parent)
    , m_Snippets(nullptr)
{
    bool success;
    LoadSnippets(success);
}

void Snippets::LoadSnippets(bool& success)
{
    success = false;
    QFile file(SNIPPETS_FILE);

    QMap<QString, QString> data = QMap<QString, QString>();

    if (file.open(QIODevice::ReadOnly)) {
        QDataStream in(&file);
        in >> data;
        file.close();
    }

    if (data.isEmpty()) {
        data.insert(tr("API Help"), tr(
                                        "# Full API\n"
                                        "# ---------------------------\n"
                                        "# get method's have no parameters and others have one\n"
                                        "#\n"
                                        "# get_input   - get input textbox's text\n"
                                        "# set_input   - set input textbox's text\n"
                                        "# get_output  - get output textbox's text\n"
                                        "# set_output  - get output textbox's text\n"
                                        "# get_code    - get code textbox's text\n"
                                        "# set_code    - set code textbox's text\n"
                                        "# write_output- append to output box\n"
                                        "# get_apppath - get exe path\n\n"
                                        "# API Help/Code Sample\n"
                                        "# ---------------------------\n"
                                        "\n"
                                        "# get text from input box\n"
                                        "# parameters - none\n"
                                        "txt = get_input()\n"
                                        "\n"
                                        "# change output box's text\n"
                                        "# parameters - string\n"
                                        "set_output(\"\")\n"
                                        "\n"
                                        "# append to output box\n"
                                        "# does not add a new line\n"
                                        "# parameters - string\n"
                                        "write_output(\"Hi You,\\n\")\n"
                                        "\n"
                                        "# get_apppath() -> get exe path\n"
                                        "print (\"PyRun.exe is at :\", get_apppath())\n\n"));
    }

    m_Snippets = new QMap<QString, QString>(data);

    success = true;
}

void Snippets::SaveSnippets(bool& success)
{
    success = false;
    if (m_Snippets != nullptr) {

        QFile file(SNIPPETS_FILE);
        if (!file.open(QIODevice::WriteOnly)) {
            return;
        }
        QDataStream out(&file);
        out << *m_Snippets;
        file.close();
        success = true;
    }
}

void Snippets::AddSnippet(const QString& name, const QString& code, bool& success)
{
    success = false;
    if (m_Snippets != nullptr) {
        success = true;
        m_Snippets->insert(name, code);
    }
}

void Snippets::RemoveSnippet(const QString& name, bool& success)
{
    success = false;
    if (m_Snippets != nullptr && m_Snippets->contains(name)) {
        success = (m_Snippets->remove(name) > 0);
    }
}

QString Snippets::GetSnippet(const QString& name, bool& success)
{
    success = false;
    if (m_Snippets != nullptr && m_Snippets->contains(name)) {
        success = true;
        return m_Snippets->value(name);
    }
    return QString();
}

QList<QString> Snippets::GetKeys(bool& success)
{
    success = false;
    if (m_Snippets != nullptr) {
        success = true;
        return m_Snippets->keys();
    }
    return QList<QString>();
}

Snippets::~Snippets()
{
    if (m_Snippets != nullptr) {
        bool success;
        SaveSnippets(success); // save on the destructor
        if (!success) {
            std::cerr << "Writing Snippets to Database on save failed" << std::endl;
        }
        delete m_Snippets;
    }
}

snippets.h

#ifndef SNIPPETS_H
#define SNIPPETS_H

#include <QObject>
#include <QMap>
#include <QList>
#include <QIODevice>
#include <QFile>
#include <QApplication>

#define SNIPPETS_FILE QApplication::applicationDirPath() + "/snippets.dat"

class Snippets : public QObject {
    Q_OBJECT
public:
    explicit Snippets(QObject* parent = 0);

    QString GetSnippet(const QString& name, bool& success);
    void RemoveSnippet(const QString& name, bool& success);
    void AddSnippet(const QString& name, const QString& code, bool& success);
    void SaveSnippets(bool& success);
    void LoadSnippets(bool& success);
    ~Snippets();
    QList<QString> GetKeys(bool& success);
signals:

public slots:

private:
    QMap<QString, QString>* m_Snippets;
};

#endif // SNIPPETS_H

What to review:

  • Best practices in C++, C++11, Qt5, and anything else

Versions/settings:

  • Qt5.3.2
  • mingw482_32
  • QMAKE_CXXFLAGS += -std=c++11
\$\endgroup\$
6
\$\begingroup\$

Not being a frequent Qt user, I don't have much to say. But there a few things:

  • SNIPPETS_FILE: That would be a lot better as a function. Macros and C++ are not a very good match. Macros have quite a few drawback, the most annoying ones are probably not respecting scope and being able to silently redefine them by mistake. I'm not saying that macros don't have any use whatsoever, but C++ offers better alternatives to most cases. A simple function (possibly inline) would be the cleanest solution for this specific case:

    inline QString GetSnippetsFileName()
    {
        return QApplication::applicationDirPath() + "/snippets.dat";
    }
    
  • Perhaps you could use a smart pointer for m_Snippets to make your code more exception safe and free yourself from the burden of deallocating the object by hand. But then again, is it really necessary to dynamically allocate m_Snippets? Why don't you declared it by value. Avoid a dynamic memory allocation where an instance declared by value will do.

  • The way you are returning the result of functions in SaveSnippets(), LoadSnippets() and others, by passing a bool& success to the function, is quite unusual. You should be returning that boolean as the function's return value instead. It would be a lot more conventional. Also, you could consider throwing and exception in some cases.

  • I don't think all of the #includes in the header file are necessary. QIODevice and QFile are probably only required by the class implementation in the .cpp file. Don't import those dependencies to all users of the class if they are not needed for someone including snippets.h.

\$\endgroup\$
  • 1
    \$\begingroup\$ Could you please add some references to the first point ? Not because I think it's wrong, I want to do some reading on it. And could you please elaborate more on the second point as well ? Thanks. \$\endgroup\$ – bhathiya-perera Nov 27 '14 at 11:12
  • 2
    \$\begingroup\$ @JaDogg - Sure, there's a lot of discussion about it on SE. Read these links for now. I'll edit the answer with more later on. link1, link2 \$\endgroup\$ – glampert Nov 27 '14 at 11:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.