Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have the following overloaded functions. All the code is the same as except for their format (%s v.s %lu), so I couldn't use templates.

Should I implement generic programming like this?

bool f(const string& key, const string& value)
{
    // some codes for input check

    exec_cmd("cmd %s %s", key.c_str(), value.c_str());

    // some codes for making return value
}

bool f(const string& key, unsigned long value)
{
    // some codes for input check

    exec_cmd("cmd %s %lu", key.c_str(), value);

    // some codes for making return value
}
share|improve this question

closed as off-topic by Jamal Dec 29 '14 at 19:20

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.

2  
You don't need generic programming for this. Using a std::stringstream will pretty much solve this problem for you. –  Yuushi Apr 4 '14 at 5:30

2 Answers 2

up vote 5 down vote accepted

I'd avoid the problem completely by building the string using a stringstream, then passing a complete result to redisCommand. There are a number of ways to do this. One that works fairly nicely for this sort of situation is to create a manipulator to actually execute your command, then pass it to a stringstream:

#include <sstream>
#include <iostream>

using std::ostringstream;
using std::ostream;

// a manipulator to execute the string in a stringstream:
ostream &exec(ostream &s) {
    ostringstream &os = dynamic_cast<ostringstream>(s);

    redisCommand(os.str().c_str());
    return s;
}

// An operator to accept and invoke the manipulator:
ostream &operator<<(ostream &s, ostream &(*manip)(ostream &)) {
    return manip(s);
}

With those in place, we can write a small template that lets us pass either type for the second parameter, and handle things from there:

template <class T>    
bool set(string const &key, T value) {   
    // ...
    ostringstream cmd;
    cmd << "SET " << key << " " << value << exec;    
    // ...
}

Of course, if you want to also allow a variable number of parameters of varying types, you can turn this into a variadic template to allow that as well (but it's a little more complex, and not immediately obvious that it's necessary, so I'll refrain for now).

share|improve this answer

If possible, the command should be built as a list of strings. Concatenating your arguments into one long command string makes me suspicious, because it converts structured data into a degraded form. For example, if any key or value happens to contain a space, then it will appear to be two arguments.


Advice below applies to Rev 5 of the question:

You haven't provided enough detail in the question to tell what you're using the result for, but I'm going to guess that it will be a shell command to be executed. In that case, there may be all kinds of security issues as well, if a key or value contains characters with special significance to the shell, such as > (redirection to a file) or ; (delimiter between two shell commands). If a hostile user is able to specify key or value, then you have an arbitrary command execution vulnerability. You can avoid this problem by using the execv() family of functions, which take an array of arguments, instead of the system() function, which expects one concatenated string.

share|improve this answer

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