STL: Could you make this snippet more efficient

[pedagani]

Dear comp.lang.c++,

Could you make this snippet more efficient? As you see I have too many
variables introduced in the code.

//Read set of integers from a file on line by line basis in a STL set
//fp is pre-defined
for(;!fp.eof(); )
{
string linestr;
getline(fp,line str);
istringstream strstreamline(l inestr);
istream_iterato r<unsigned intintstream(st rstreamline);
set<unsigned intpckset;
copy ( intstream , istream_iterato r<unsigned int>() ,
inserter(modpck set,modpckset.b egin ( ) ));
}

Thanks in advance.

[Victor Bazarov]

pe******@gmail. com wrote:

Dear comp.lang.c++,

Could you make this snippet more efficient? As you see I have too many
variables introduced in the code.

//Read set of integers from a file on line by line basis in a STL set
//fp is pre-defined
for(;!fp.eof(); )
{
string linestr;
getline(fp,line str);
istringstream strstreamline(l inestr);
istream_iterato r<unsigned intintstream(st rstreamline);
set<unsigned intpckset;

What's that for?

copy ( intstream , istream_iterato r<unsigned int>() ,
inserter(modpck set,modpckset.b egin ( ) ));
}

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[pedagani]

On Dec 3, 11:37 am, "Victor Bazarov" <v.Abaza...@com Acast.netwrote:

pedag...@gmail. com wrote:

Dear comp.lang.c++,

Could you make this snippet more efficient? As you see I have too many
variables introduced in the code.

//Read set of integers from a file on line by line basis in a STL set
//fp is pre-defined
for(;!fp.eof(); )
{
string linestr;
getline(fp,line str);
istringstream strstreamline(l inestr);
istream_iterato r<unsigned intintstream(st rstreamline);
set<unsigned intpckset;

What's that for?

copy ( intstream , istream_iterato r<unsigned int>() ,
inserter(modpck set,modpckset.b egin ( ) ));
}

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

*Correction* 'pckset' should have been 'modpckset'.

[Victor Bazarov]

pe******@gmail. com wrote:

On Dec 3, 11:37 am, "Victor Bazarov" <v.Abaza...@com Acast.netwrote:

>pedag...@gmail .com wrote:

>>Dear comp.lang.c++,

>>Could you make this snippet more efficient? As you see I have too
many variables introduced in the code.

>>//Read set of integers from a file on line by line basis in a STL
set //fp is pre-defined
for(;!fp.eof( );)
{
string linestr;
getline(fp,li nestr);
istringstre am strstreamline(l inestr);
istream_iterato r<unsigned intintstream(st rstreamline);
set<unsigne d intpckset;

What's that for?

>>copy ( intstream , istream_iterato r<unsigned int>() ,
inserter(modp ckset,modpckset .begin ( ) ));
}

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

*Correction* 'pckset' should have been 'modpckset'.

OK. The entire snippet can be made more efficient by removing
the lines between

getline(fp, linestr);

and

}

; they do essentially no work that would have side effects.
The creation of a stringstream, reading stuff from it, and
putting those unsigned ints into a local set are a waste of
CPU cycles.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[red floyd]

pe******@gmail. com wrote:

Dear comp.lang.c++,

Could you make this snippet more efficient? As you see I have too many
variables introduced in the code.

//Read set of integers from a file on line by line basis in a STL set
//fp is pre-defined
for(;!fp.eof(); )

Hint. The for statement does not do what you think it does.
Also, why are you using a for() instead of a while() in this situation?

{
string linestr;
getline(fp,line str);
istringstream strstreamline(l inestr);
istream_iterato r<unsigned intintstream(st rstreamline);
set<unsigned intpckset;
copy ( intstream , istream_iterato r<unsigned int>() ,
inserter(modpck set,modpckset.b egin ( ) ));
}

Thanks in advance.

[pedagani]

On Dec 3, 11:50 am, "Victor Bazarov" <v.Abaza...@com Acast.netwrote:

pedag...@gmail. com wrote:

On Dec 3, 11:37 am, "Victor Bazarov" <v.Abaza...@com Acast.netwrote:

pedag...@gmail. com wrote:
Dear comp.lang.c++,

>Could you make this snippet more efficient? As you see I have too
many variables introduced in the code.

>//Read set of integers from a file on line by line basis in a STL
set //fp is pre-defined
for(;!fp.eof() ;)
{
string linestr;
getline(fp,lin estr);
istringstrea m strstreamline(l inestr);
istream_iterato r<unsigned intintstream(st rstreamline);
set<unsigned intpckset;

What's that for?

>copy ( intstream , istream_iterato r<unsigned int>() ,
inserter(modpc kset,modpckset. begin ( ) ));
}

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

*Correction* 'pckset' should have been 'modpckset'.

OK. The entire snippet can be made more efficient by removing
the lines between

getline(fp, linestr);

and

}

; they do essentially no work that would have side effects.
The creation of a stringstream, reading stuff from it, and
putting those unsigned ints into a local set are a waste of
CPU cycles.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

Insightful perspective. It just me that I dont have to necessarily
deal them as integers and could consider them as strings instead.
Appreciate your help.

However, I need to store the parsed result in a container on which I
could apply set_difference algorithm. Looks like I must resort to
stringstream in this case. Please correct me if I am wrong.

Best.

[pedagani]

On Dec 3, 11:51 am, red floyd <no.s...@here.d udewrote:

pedag...@gmail. com wrote:

Dear comp.lang.c++,

Could you make this snippet more efficient? As you see I have too many
variables introduced in the code.

//Read set of integers from a file on line by line basis in a STL set
//fp is pre-defined
for(;!fp.eof(); )

Hint. The for statement does not do what you think it does.
Also, why are you using a for() instead of a while() in this situation?

{
string linestr;
getline(fp,line str);
istringstream strstreamline(l inestr);
istream_iterato r<unsigned intintstream(st rstreamline);
set<unsigned intpckset;
copy ( intstream , istream_iterato r<unsigned int>() ,
inserter(modpck set,modpckset.b egin ( ) ));
}

Thanks in advance.

Thank you, got it.

[Victor Bazarov]

pe******@gmail. com wrote:

On Dec 3, 11:50 am, "Victor Bazarov" <v.Abaza...@com Acast.netwrote:

>pedag...@gmail .com wrote:

>>On Dec 3, 11:37 am, "Victor Bazarov" <v.Abaza...@com Acast.net>
wrote:
pedag...@gma il.com wrote:
Dear comp.lang.c++,

>>>>Could you make this snippet more efficient? As you see I have too
many variables introduced in the code.

>>>>//Read set of integers from a file on line by line basis in a STL
set //fp is pre-defined
for(;!fp.eo f();)
{
string linestr;
getline(fp, linestr);
istringstre am strstreamline(l inestr);
istream_iterato r<unsigned intintstream(st rstreamline);
set<unsigne d intpckset;

>>>What's that for?

>>>>copy ( intstream , istream_iterato r<unsigned int>() ,
inserter(mo dpckset,modpcks et.begin ( ) ));
}

>>>V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

>>*Correction * 'pckset' should have been 'modpckset'.

OK. The entire snippet can be made more efficient by removing
the lines between

getline(fp, linestr);

and

}

; they do essentially no work that would have side effects.
The creation of a stringstream, reading stuff from it, and
putting those unsigned ints into a local set are a waste of
CPU cycles.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

Insightful perspective. It just me that I dont have to necessarily
deal them as integers and could consider them as strings instead.
Appreciate your help.

However, I need to store the parsed result in a container on which I
could apply set_difference algorithm. Looks like I must resort to
stringstream in this case. Please correct me if I am wrong.

If you have to convert them, you should use some kind of conversion
function, be it strtod, sscanf, or istream::operat or>>. Actually,
it is quite possible that operator>intern ally uses strtod.

If storing them in a set is a requirements, you probably need to
do your 'copy' with the second argument being 'inserter' for some
other, non-local set.

It is possible that once the string is read, you might have better
luck using 'strtod' yourself (essentially replacing what operator>>
does, skipping WS, etc.) However, without measuring it's hard to
say what area of your algorithm you need to address.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[Mark P]

pe******@gmail. com wrote:

Dear comp.lang.c++,

Could you make this snippet more efficient? As you see I have too many
variables introduced in the code.

//Read set of integers from a file on line by line basis in a STL set
//fp is pre-defined
for(;!fp.eof(); )
{
string linestr;
getline(fp,line str);
istringstream strstreamline(l inestr);
istream_iterato r<unsigned intintstream(st rstreamline);
set<unsigned intpckset;
copy ( intstream , istream_iterato r<unsigned int>() ,
inserter(modpck set,modpckset.b egin ( ) ));
}

Efficiency doesn't have much to do with number of variables, however, it
probably *is* more efficient to stream your results into a vector, sort
it, and then apply set_difference (which, of course, doesn't actually
require a std::set). One sort at the end is likely to be faster than
maintaining a sorted structure throughout the construction.

Mark

[Daniel T.]

pe******@gmail. com wrote:

Could you make this snippet more efficient? As you see I have too many
variables introduced in the code.

//Read set of integers from a file on line by line basis in a STL set
//fp is pre-defined
for(;!fp.eof(); )
{
string linestr;
getline(fp,line str);
istringstream strstreamline(l inestr);
istream_iterato r<unsigned intintstream(st rstreamline);
set<unsigned intpckset;
copy ( intstream , istream_iterato r<unsigned int>() ,
inserter(modpck set,modpckset.b egin ( ) ));
}

Thanks in advance.

First let's clean up the code you have.

set< unsigned int modpckset; // has to be defined outside the loop
string line;
while ( getline( fp, line ) ) // proper way to read until end of file
{
istringstream ss( line );
copy( istream_iterato r< unsigned int >( ss ),
istream_iterato r< unsigned int >(),
inserter( modpckset, modpckset.begin () ) );
}

With the above code, getline will read in a line of the file at a time,
then parse it out to a bunch of unsigned ints. We don't need to do the
extra step:

set< unsigned int modpckset;
copy( istream_iterato r< unsigned int >( fp ),
istream_iterato r< unsigned int >(),
inserter( modpckset, modpckset.begin () ) );

The above will do the same thing.

Now, if your goal was to read each line into a separate set, say you
want to end up with a vector of sets, then you would need to read each
line separately.

vector< set< unsigned int all_sets;
string line;
while ( getline( fp, line ) )
{
istringstream ss( line );
all_sets.push_b ack( set< unsigned int >() );
copy( istream_iterato r< unsigned int >( ss ),
istream_iterato r< unsigned int >(),
inserter( all_sets.back() , all_sets.back() .begin() ) );
}