vector of struct ?

Discussion in 'C++' started by sd2004, Dec 8, 2005.

  1. sd2004

    sd2004 Guest

    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
    sd2004, Dec 8, 2005
    #1
    1. Advertising

  2. sd2004

    Ron Natalie Guest

    sd2004 wrote:
    > 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.
    >
    Ron Natalie, Dec 8, 2005
    #2
    1. Advertising

  3. sd2004

    mlimber Guest

    sd2004 wrote:
    > 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
    mlimber, Dec 8, 2005
    #3
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. pmatos
    Replies:
    6
    Views:
    23,739
  2. Chris Fogelklou
    Replies:
    36
    Views:
    1,358
    Chris Fogelklou
    Apr 20, 2004
  3. Replies:
    8
    Views:
    1,896
    Csaba
    Feb 18, 2006
  4. Javier
    Replies:
    2
    Views:
    549
    James Kanze
    Sep 4, 2007
  5. Rushikesh Joshi
    Replies:
    0
    Views:
    349
    Rushikesh Joshi
    Jul 10, 2004
Loading...

Share This Page