patterncppModerate
Simple server to manipulate priority queues
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)
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:
The
The
This is what it could look like (using your existing style):
Even the inner
I've also noticed a lack of curly braces with your other
Related: curly braces and Apple's SSL bug
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.