URL Query params builder

Question

My goal is to create a URL query params from std::map<std::string, std::string>. For example, we call to_query_params( {{"one","1"},{"two","2"}} ) and we get ?one=1&two=2.

Is this a good way of doing it?

std::string to_query_params(std::map<std::string, std::string> map)
{
 if (map.empty()){
 return "";
 }
 auto map_beg = std::begin(map);
 auto concat = [](const decltype(map)::value_type& a){
 return a.first+"="+a.second;
 };
 std::string result("?"+concat(*map_beg));
 std::for_each(std::next(std::begin(map)), 
 std::end(map), 
 [&](decltype(map)::value_type param){ 
 result+="&"+concat(param); 
 });
 return result;
}

• SuperBiasedMan: What kind of feedback are you interested in? Performance, readability, general feedback?
• jaor: Performance, readability and maintenance.

Answer 1

You are not performing any percent encoding of parameter names or values, which would likely lead to breakage or even a security vulnerability if the you are given parameters that contain a special character.

A std::map might not be the right type; a std::multimap may be more appropriate.

Answer 2

• Give your operators some breathing space:

  return a.first + "=" + a.second;
  

• Concatenation involves constructing and deleting many temporary strings. std::stringstream avoids it:

  std::stringstream result;
  ...
  result << '&' << a.first << '=' << a.second;
  ...
  return result.str();
  

• Pass map by reference.

Answer 3

There is no reason to use decltype

auto concat = [](const decltype(map)::value_type& a){
 return a.first+"="+a.second;
 };

Why not use auto

auto concat = [](auto const& a){
 return a.first+"="+a.second;
 };

Don't special case the '?' Just use '&' then change the one character after you have finished.

std::string to_query_params(std::map<std::string, std::string> map)
{
 if (map.empty()){
 return "";
 }
 std::string result;
 std::for_each(std::begin(map), 
 std::end(map), 
 [&](auto const& param){ 
 result+="&"+concat(param); 
 });
 // Just convert the first '&' into '?'
 result[0] = '?';
 return result;
}

The reason for this is that you use different techniques for both cases by only using one you make sure your code is DRY and consistent.

 // Version 1 parameter:
 const decltype(map)::value_type&
 // Version 2 parameter:
 decltype(map)::value_type // different.

Also:

 // Vesion 1
 return a.first+"="+a.second;
 // Version 2:
 result+="&"+concat(param); // concat or + "="

But rather than using a lambda we can use a range based for loop.

 std::string result;
 std::for(auto const& value: map) 
 result += "&" + concat(value); 
 );

I would probably use a std::map<std::string, std::vector<std::string>> that's because parameters in the URL allow multiple values and their order is significant.

• jaor: "Why not use auto in lambda" - because it's c++11 :)