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,linestr);
istringstream strstreamline(linestr);
istream_iterator<unsigned intintstream(strstreamline);
set<unsigned intpckset;
copy ( intstream , istream_iterator<unsigned int>() ,
inserter(modpckset,modpckset.begin ( ) ));
}

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,linestr);
istringstream strstreamline(linestr);
istream_iterator<unsigned intintstream(strstreamline);
set<unsigned intpckset;

What's that for?

copy ( intstream , istream_iterator<unsigned int>() ,
inserter(modpckset,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

[pedagani]

On Dec 3, 11:37 am, "Victor Bazarov" <[e-mail]:

[e-mail] 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,linestr);
istringstream strstreamline(linestr);
istream_iterator<unsigned intintstream(strstreamline);
set<unsigned intpckset;

What's that for?

copy ( intstream , istream_iterator<unsigned int>() ,
inserter(modpckset,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'.

[Victor Bazarov]

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

On Dec 3, 11:37 am, "Victor Bazarov" <[e-mail]:

>[e-mail] 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,linestr);
istringstream strstreamline(linestr);
istream_iterator<unsigned intintstream(strstreamline);
set<unsigned intpckset;

What's that for?

>>copy ( intstream , istream_iterator<unsigned int>() ,
inserter(modpckset,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,linestr);
istringstream strstreamline(linestr);
istream_iterator<unsigned intintstream(strstreamline);
set<unsigned intpckset;
copy ( intstream , istream_iterator<unsigned int>() ,
inserter(modpckset,modpckset.begin ( ) ));
}

Thanks in advance.

[pedagani]

On Dec 3, 11:50 am, "Victor Bazarov" <[e-mail]:

[e-mail] wrote:

On Dec 3, 11:37 am, "Victor Bazarov" <[e-mail]:

[e-mail] 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,linestr);
istringstream strstreamline(linestr);
istream_iterator<unsigned intintstream(strstreamline);
set<unsigned intpckset;

What's that for?

>copy ( intstream , istream_iterator<unsigned int>() ,
inserter(modpckset,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 <[e-mail]:

[e-mail] 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,linestr);
istringstream strstreamline(linestr);
istream_iterator<unsigned intintstream(strstreamline);
set<unsigned intpckset;
copy ( intstream , istream_iterator<unsigned int>() ,
inserter(modpckset,modpckset.begin ( ) ));
}

Thanks in advance.

Thank you, got it.

[Victor Bazarov]

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

On Dec 3, 11:50 am, "Victor Bazarov" <[e-mail]:

>[e-mail] wrote:

>>On Dec 3, 11:37 am, "Victor Bazarov" <[e-mail]>
wrote:
[e-mail] 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,linestr);
istringstream strstreamline(linestr);
istream_iterator<unsigned intintstream(strstreamline);
set<unsigned intpckset;

>>>What's that for?

>>>>copy ( intstream , istream_iterator<unsigned int>() ,
inserter(modpckset,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.

If you have to convert them, you should use some kind of conversion
function, be it strtod, sscanf, or istream::operator>>. Actually,
it is quite possible that operator>internally 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,linestr);
istringstream strstreamline(linestr);
istream_iterator<unsigned intintstream(strstreamline);
set<unsigned intpckset;
copy ( intstream , istream_iterator<unsigned int>() ,
inserter(modpckset,modpckset.begin ( ) ));
}

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,linestr);
istringstream strstreamline(linestr);
istream_iterator<unsigned intintstream(strstreamline);
set<unsigned intpckset;
copy ( intstream , istream_iterator<unsigned int>() ,
inserter(modpckset,modpckset.begin ( ) ));
}

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_iterator< unsigned int >( ss ),
istream_iterator< 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_iterator< unsigned int >( fp ),
istream_iterator< 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_back( set< unsigned int >() );
copy( istream_iterator< unsigned int >( ss ),
istream_iterator< unsigned int >(),
inserter( all_sets.back(), all_sets.back().begin() ) );
}

[pedagani]

On Dec 3, 3:23 pm, "Daniel T." <[e-mail]:

[e-mail] 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,linestr);
istringstream strstreamline(linestr);
istream_iterator<unsigned intintstream(strstreamline);
set<unsigned intpckset;
copy ( intstream , istream_iterator<unsigned int>() ,
inserter(modpckset,modpckset.begin ( ) ));
}

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_iterator< unsigned int >( ss ),
istream_iterator< 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_iterator< unsigned int >( fp ),
istream_iterator< 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_back( set< unsigned int >() );
copy( istream_iterator< unsigned int >( ss ),
istream_iterator< unsigned int >(),
inserter( all_sets.back(), all_sets.back().begin() ) );
}

Very nice. If its not too much clutter one could do the same as

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
{
copy( istream_iterator< unsigned int >( istringstream (line) ),
istream_iterator< unsigned int >(),
inserter( modpckset, modpckset.begin() ) );
//...
}

[Victor Bazarov]

Daniel T. wrote:

[..]
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_iterator< unsigned int >( ss ),
istream_iterator< 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_iterator< unsigned int >( fp ),
istream_iterator< unsigned int >(),
inserter( modpckset, modpckset.begin() ) );

The above will do the same thing.

Really? Will it? What if it can't copy (there is 'O' instead of '0'
somewhere in the file)? Will it attempt to read again from the next
line?

[..]

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

[Daniel T.]

"Victor Bazarov" <v.********@comAcast.netwrote:

Daniel T. wrote:

[..]
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_iterator< unsigned int >( ss ),
istream_iterator< 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_iterator< unsigned int >( fp ),
istream_iterator< unsigned int >(),
inserter( modpckset, modpckset.begin() ) );

The above will do the same thing.

Really? Will it? What if it can't copy (there is 'O' instead of '0'
somewhere in the file)? Will it attempt to read again from the next
line?

Sorry, at some point while writing the post, I had written that I was
assuming that the file contained all unsigned ints. I must have
accidentally cut that line out.