HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Snippets Manager

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
snippetsmanagerstackoverflow

Problem

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

snippets.cpp

```
#include "Features/snippets.h"
#include

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 data = QMap();

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

Solution

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.

Code Snippets

inline QString GetSnippetsFileName()
{
    return QApplication::applicationDirPath() + "/snippets.dat";
}

Context

StackExchange Code Review Q#70877, answer score: 6

Revisions (0)

No revisions yet.