vector of struct ?

Coudl someone make the following code more elegant ?
#include<iostream>
#include <string>
#include<vector>
#include<sstream>
#include<fstream>
using namespace std;

struct astruct
{
string name;
int id;
};

int main()
{
vector<astruct> v;
astruct astr;
astruct* sp;

ifstream in ("test5.txt");
string line;
while (getline(in,line)){
istringstream anyname(line);
anyname>>astr.name>>astr.id;
v.push_back(astr);
}

sp=&v[0];
for (sp=&v[0];sp->name!="EOT";sp++){
cout<<"Name : "<<sp->name<<" "<<"ID: "<<sp->id<<endl;
}
return 0;

}
///////////////////////input file =test5.txt
///////////////////////////////
Tom 777
Idaho 555
China 111
Cricket 333
EOT

re: vector of struct ?

sd2004 wrote:[color=blue]
> Coudl someone make the following code more elegant ?[/color]
[color=blue]
> using namespace std;
>[/color]
avoid dumping std into the global namespace. Someday you'll want to
make this class accessible to others presumably via an include file
where such is considered anti-social behavior.
[color=blue]
> struct astruct
> {
> string name;
> int id;
> };[/color]

I detest uninitialized variables. Provide a constructor to set
id.
[color=blue]
> astruct astr;[/color]
This is declared outside the more restrictive scope (the while loop)
where it is used.[color=blue]
> astruct* sp;[/color]
Same here, plus it's left uninitialized. You could have declared it in
the for initializer itself.
[color=blue]
>
> ifstream in ("test5.txt");[/color]

You need to test to see if this fails.
[color=blue]
> string line;
> while (getline(in,line)){
> istringstream anyname(line);
> anyname>>astr.name>>astr.id;
> v.push_back(astr);
> }[/color]
You could have defined a member function for astruct to read a line and
set the variable. This might even end up being more efficient than
pushing back a COPY of the struct. You could just grow the vector and
then call the input function on the object that is already residing in
the vector.[color=blue]
>
> sp=&v[0];
> for (sp=&v[0];sp->name!="EOT";sp++){[/color]

I'm not sure why you init sp twice. std::find will perform a similar
operation.
[color=blue]
> cout<<"Name : "<<sp->name<<" "<<"ID: "<<sp->id<<endl;[/color]

You could overload ostream& << for this class.[color=blue]
> }
> return 0;
>
> }
> ///////////////////////input file =test5.txt
> ///////////////////////////////
> Tom 777
> Idaho 555
> China 111
> Cricket 333
> EOT[/color]

Given this data, I'd not even stick EOT in the vector. Vectors are
already keeping track of the size so it's kind of pointless.[color=blue]
>[/color]

re: vector of struct ?

sd2004 wrote:[color=blue]
> Coudl someone make the following code more elegant ?
> #include<iostream>
> #include <string>
> #include<vector>
> #include<sstream>
> #include<fstream>
> using namespace std;
>
> struct astruct
> {
> string name;
> int id;
> };
>[/color]

istream& operator >> ( istream& is, const astruct& s )
{
return is >> s.name >> s.id;
}

ostream& operator << ( ostream& os, const astruct& s )
{
return os << s.name << ' ' << s.id;
}
[color=blue]
> int main()
> {
> vector<astruct> v;
> astruct astr;
> astruct* sp;[/color]

Don't declare variables until you can initialize and use them.
[color=blue]
>
> ifstream in ("test5.txt");
> string line;
> while (getline(in,line)){
> istringstream anyname(line);
> anyname>>astr.name>>astr.id;
> v.push_back(astr);
> }[/color]

Try:

vector<astruct> v;
ifstream in ("test5.txt");
astruct astr;
while( in >> astr )
{
v.push_back( astr );
}
[color=blue]
> sp=&v[0];
> for (sp=&v[0];sp->name!="EOT";sp++){
> cout<<"Name : "<<sp->name<<" "<<"ID: "<<sp->id<<endl;
> }[/color]

Get rid of the EOT at the end if you have control over file format, and
then:

copy( v.begin(), v.end(), ostream_iterator<astruct>( cout, "\n" ) );

You'll need to #include <algorithm> and <iterator>.

If you can't change the file format, do this:

typedef vector<astruct>::const_iterator CI;
for( CI i = v.begin(); i != v.end() && i->name != "EOT"; ++i )
{
cout << *i << endl;
}
[color=blue]
> return 0;
>
> }
> ///////////////////////input file =test5.txt
> ///////////////////////////////
> Tom 777
> Idaho 555
> China 111
> Cricket 333
> EOT[/color]

Cheers! --M