vector of struct ?

[sd2004]

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

 

[Ron Natalie]

Coudl someone make the following code more elegant ?

using namespace std;

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.

struct astruct
{
string name;
int id;
};

I detest uninitialized variables. Provide a constructor to set
id.

astruct astr;

This is declared outside the more restrictive scope (the while loop)
where it is used.

astruct* sp;

Same here, plus it's left uninitialized. You could have declared it in
the for initializer itself.

ifstream in ("test5.txt");

You need to test to see if this fails.

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

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.

sp=&v[0];
for (sp=&v[0];sp->name!="EOT";sp++){

I'm not sure why you init sp twice. std::find will perform a similar
operation.

cout<<"Name : "<<sp->name<<" "<<"ID: "<<sp->id<<endl;

You could overload ostream& << for this class.

}
return 0;

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

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.

 

[mlimber]

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;
};

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;
}

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

Don't declare variables until you can initialize and use them.

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

Try:

vector<astruct> v;
ifstream in ("test5.txt");
astruct astr;
while( in >> astr )
{
v.push_back( astr );
}

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

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;
}

return 0;

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

Cheers! --M