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

Simple server to manipulate priority queues

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

Problem

In a few months I'll work on a C++ project that could span a year or two. I've a strong C background and for the usual reasons I always chose C over C++ (please forgive me). To extend my capabilities and to move from C to some more powerful language, I felt the time has come to take a serious step towards C++. In order to do so I have created a small C++ project just for myself.
I often read that C programmers do not use the power C++ and all of its glory has to offer, something I madly tried to avoid. I would like some code reflection, points of improvement and other tips. Any suggestions or enhancements are welcome.

Please note that the code below is only to illustrate my C++ programming skills. Although the project will compile (and run) is has no purpose, neither does it really do anything. Just a learning experience...

```
#include
#include
#include
#include
#include
#include
#include

#include "queue.h"

using namespace std;

static list listQueue;

void tokenizeCmd(const string &str, vector &tokens, const string &delimiters = " ") {
string::size_type lastPos = str.find_first_not_of(delimiters, 0);
string::size_type pos = str.find_first_of(delimiters, lastPos);

while (string::npos != pos || string::npos != lastPos) {
tokens.push_back(str.substr(lastPos, pos - lastPos));
lastPos = str.find_first_not_of(delimiters, pos);
pos = str.find_first_of(delimiters, lastPos);
}
}

void processCommand(string strCmd) {
vector tokens;
tokenizeCmd(strCmd, tokens);

if (tokens.empty())
return;

try {
if(tokens.at(0) == "list") {
cout ::iterator it = listQueue.begin(); it != listQueue.end(); ++it)
std::cout GetName() GetPriority() 2)
cA.SetPriority(std::stoi(tokens.at(2)));
listQueue.push_back(cA);

} else if(tokens.at(0) == "remove") {
for(std::list::iterator it = listQueue.begin(); it != listQueue.end(); ++it)

Solution

The lack of curly braces here has a potential for errors:

if (argc < 2)
    /*TODO daemon */
    cout << "Starting server\n";
else
    for(int i=1; i<argc; i++)
        if (!strcmp(argv[i], "-s"))
            shell();
        else if (!strcmp(argv[i], "-s"))
            exit(0);
        else
            usage(argv[0]);


The if statement may execute the output statement, but if you were to put some code on the same line as that comment, it would no longer execute the previous statement.

The else and the for should really have curly braces. This may confuse others and could still break if changes are made carelessly.

This is what it could look like (using your existing style):

if (argc < 2) {
    /*TODO daemon */
    cout << "Starting server\n";
} else {
    for (int i=1; i<argc; i++) {
        if (!strcmp(argv[i], "-s"))
            shell();
        else if (!strcmp(argv[i], "-s"))
            exit(0);
        else
            usage(argv[0]);
    }
}


Even the inner if/else statements should have curly braces, but you get the idea.

I've also noticed a lack of curly braces with your other for loops. Make sure this is applied everywhere to minimize error-prone code.

Related: curly braces and Apple's SSL bug

Code Snippets

if (argc < 2)
    /*TODO daemon */
    cout << "Starting server\n";
else
    for(int i=1; i<argc; i++)
        if (!strcmp(argv[i], "-s"))
            shell();
        else if (!strcmp(argv[i], "-s"))
            exit(0);
        else
            usage(argv[0]);
if (argc < 2) {
    /*TODO daemon */
    cout << "Starting server\n";
} else {
    for (int i=1; i<argc; i++) {
        if (!strcmp(argv[i], "-s"))
            shell();
        else if (!strcmp(argv[i], "-s"))
            exit(0);
        else
            usage(argv[0]);
    }
}

Context

StackExchange Code Review Q#75527, answer score: 13

Revisions (0)

No revisions yet.