# Separate method for a single line code? Or embed in the caller function?

I'm having an argument with my boss about whether or not it's better to have a separate function for a single line of code.

while (count <= poscount)
{
key = etc.substr(pos2prev+1, pos-pos2prev-1);
std::string::iterator end_pos = std::remove(key.begin(), key.end(), ' ');
key.erase(end_pos, key.end());

val = etc.substr(pos+1, pos2-pos-1);

//process key and val
DEBUG("{\n\t%s:%s\n}\n", key.c_str(), val.c_str());

count++;
pos2prev = pos2;
pos = etc.find("=", pos+1);
pos2 = (count == poscount) ? etc.find("\0",pos2+1) : etc.find("&", pos2+1);
}


Is it better coding style to include the line

json_object_object_add(paramList, key.c_str(), json_object_new_string(val.c_str()));


in a separate function name such as processKeyVal(), or leave it as-is?

I'm guessing that your boss's argument is that a well named function would make the code more readable, and I agree with that in general. I often do that myself even when the function contains a single line.

In this particular example I would not bother though. The comment above the json-call says the same thing as the function name would, and there are no obvious advantages to make a function for a single call.

So if your question is specifically for this example, then I agree with you - keep it as it is, but if you want a more general answer, then I agree with your boss.

For example - if this one-liner is something that you use in more than one place, i.e. if calling the "processKeyVal()" function in more than one place makes sense. Then I say you should make a function of it even if it would contain only one line of code.

Or to put it another way - it is not (always) the number of code lines that should determine if you make a separate function of a code snippet.

Not a direct answer to your question and possibly far fetched but here you go:

Your code looks like it's parsing a string (presumably a query string) and serializes it into JSON. I would consider extracting the json serialization dependency out of it and pass in an abstract serializer. The line in question would turn into:

m_serializer.Add(key, val);


• Separates the parsing of the code from the serialization
• Allows you to easily add different forms of serialization without having to change the parsing code
• Makes unit testing (in case you do have unit tests) cleaner because it would just test that the right methods with the right arguments are called on a mock serializer.

As it stands now I probably would not bother adding an additional method however I would introduce local variables to hold the arguments for the call instead. Can make debugging a tad easier.

• ...and readability++ :) – Mathieu Guindon Nov 17 '13 at 2:52

I would leave it as it is.

It's only 1 line, and it's a function call of itself and not an evaluation or an operation, so why wrap a function call within a separate function.

Give it its own function. Since this is C++, you can inline it so that it won't incur any performance penalty or use a dangerous macro. If you decide you want XML or a binary format or anything else tomorrow, you will be glad you didn't commit the rest of your code to JSON by using json_object_object_add.