Simplifying FOR Loops
I have a function that essentially reads the values from a vector of twins, appends them to the string (while providing space between them and setting their fixes), and returns the final result minus the trailing space:
std::string MultiplePrintProperties::GetHpitchString()
{
std::string str;
vector< double >::iterator it;
for ( it = Vals.begin();
it != Vals.end();
it++ )
{
ostringstream s;
// Set precision to 3 digits after the decimal point
// and read into the string
boost::format fmt( "%.3f " );
s << fmt % *( it );
str.append( s.str() );
}
// Remove last white space and return string
return str.substr( 0, str.length() - 1 );
}
I would like to know if this code can be simplified in any way. I've recently been looking into using for_each and functors in particular, but haven't been able to figure out how these methods can improve on this particular example.
source to share
Since you are actually converting double strings to strings and appending those strings to the string stream, you can use std::transform
to do this:
// your functor, transforming a double into a string
struct transform_one_double {
std::string operator()( const double& d ) const {
boost::format fmt( "%.3f " );
return (fmt % d).str();
}
};
// iteration code, taking each value and inserting the transformed
// value into the stringstream.
std::transform( vals.begin(), vals.end()
, std::ostream_iterator<std::string>( s, " ")
, transform_one_double() );
source to share
It seems that I am a little out of date. I would do this:
std::string MultiplePrintProperties::GetHpitchString()
{
std::string str;
vector< double >::iterator it;
for ( it = Vals.begin();
it != Vals.end();
it++ )
{
// Set precision to 3 digits after the decimal point
// and write it into the string
char buf[20];
snprintf( buf, 20, "%.3f", *it );
if (str.length() > 0)
str.append(" ");
str.append( buf );
}
return str;
}
source to share
The variable 'fmt' must be declared outside of your loop, as setting the formatting of each iteration is slow and unnecessary. also string is unnecessary. Thus, the body will become something like this:
std::string s;
std::vector<double>::iterator i = vals.begin();
if (i != vals.end())
{
boost::format fmt("%.3f");
s = str(fmt % *i++);
while (i != vals.end())
s += ' ' + str(fmt % *i++);
}
I didn't find your original code bloated or desperate for simplification. However, I would move
boost::format fmt("%.3f");
and
ostringstream s;
out of the loop to make sure they are only initialized once. This will save a lot of str.append () - too. I guess the xtofl std :: transform () solution will have this problem (it's easy to fix by initializing it once for a struct though).
If you are looking for other alternatives
for (it = begin(); it != end(); ++it) {...}
take a look at BOOST_FOREACH which will allow you to iterate like this:
std::vector<double> list;
BOOST_FOREACH(double value, list) {
...
}
source to share
You can create an overloaded class with a operator()
reference to std :: string as a member. You will declare an object of this class and pass a string to the constructor, and then use that object as the third parameter to for_each. The overloaded operator () will be called for each element and append text to the specified string.
source to share
as above, there are many ways to achieve this, but ... doesn't this method ask for a few more parameters and templating? suppose you had
template< class tType >
std::string PrintVectorToArray( const std::vector< tType >& V, const char* Seperator );
then you can create
1 2 3
1, 2, 3
1.0 and then 2.0 and then 5.0
for any type convertible to a string and with any separator! I used to do it this way, and now I use this method a lot.
source to share
I would suggest using one string stream and one format. It's not exactly cheap.
std::string MultiplePrintProperties::GetHpitchString()
{
std::ostringstream s;
// Set precision to 3 digits after the decimal point
static boost::format fmt( "%.3f " );
for ( vector< double >::iterator it = Vals.begin();
it != Vals.end(); it++ )
{
// and read into the string
s << fmt % *( it );
}
// Remove last white space (if present) and return result
std::string ret = s.str();
if (!ret.empty()) ret.resize(ret.size()-1);
return ret;
}
If I had some profiling information to prove that this was still a bottleneck, I would consider using a static ostringstream:
static std::ostringstream s;
...
std::string ret;
std::swap(ret, s.str());
return ret;
source to share