Processing and reading a trajectory file efficiently using OOP

Question

I am writing a code to read in large .xyz files. These type of files are useful for Molecular Dynamics visualizers like VMD. So the file format looks something like this

#Number of particles
#frame number
#Coordinates

As an example:

5
0
C    1.23    2.33    4.56
C    1.23    2.33    5.56
C    1.23    2.33    6.56
C    1.23    2.33    7.56
C    1.23    2.33    8.56
5
1
C    2.23    2.33    4.56
C    2.23    3.33    5.56
C    2.23    4.33    6.56
C    2.23    5.33    7.56
C    2.23    6.33    8.56

and so on. I was trying to understand this post here https://codereview.stackexchange.com/questions/201743/processing-xyz-data-from-a-large-file which talks about efficiently reading from large datasets using operator overloading method. I am trying to write a class which can read such large trajectory files and give me the following outputs : 1) number of particles 2) Total number of frames 3)set of Coordinates at each time step. So i have tried to write down the following based on this post to read in the the file format as mentioned above. So far the code below is able to read a single frame and exits after that.

#include <iostream>
#include <vector>
#include <fstream>

struct Particle{

    long double x,y,z;
    char tab ='\t';
    char newline = '\n';
    char atom ;
    friend std::istream& operator>>(std::istream& in, Particle &xyz) {
        in >> xyz.atom >> xyz.x >> xyz.y >> xyz.z ;
        return in;
    }
    friend std::ostream& operator<<(std::ostream& out, Particle &xyz){
        out << xyz.x << xyz.tab << xyz.y << xyz.tab << xyz.z << xyz.newline;
        return out;
    }
};
class XYZ_frame_read
{

    int curr_frame;
    int num_particles;
    std::vector<Particle> coordinates_t;

    public:

    friend std::istream& operator>>(std::istream& in, XYZ_frame_read &traj ){

                in >> traj.num_particles;
                in >> traj.curr_frame;
                Particle p;
                while(in >> p){
                    traj.coordinates_t.push_back(p);
                }
            return in;
        }
    friend std::ostream& operator<<(std::ostream& out, XYZ_frame_read &traj){

            for(int i = 0; i< traj.num_particles ;i ++){
                out << traj.coordinates_t.at(i) ;
            }
            return out;
        }
};

int main(int argc, char *argv[]){

    std::ifstream in(argv[1]);
    XYZ_frame_read* frames = new XYZ_frame_read[3];
    in >> frames[0];
    std::cout << frames[0];

    return 0;
}

The problem is I don't understand how will I will implement this method to read the next frames and keep appending them to the coordinates_t vector for each instance of the object XYZ_frame_read. I think I understand how this works so obviously a while(!in.eof()) is out of question, since it'll just read the first frame over and over again. I am a newbie to c++ and am working on a Molecular dyanamics related project, any changes/suggestions are welcome!! Thank you for the help!

EDIT

I have tried using

size_t i = 0;
while(in >> frames[i]){
    std::cout << frames[i];
    if(i == 3){
        break;
    }
    i++;
}

It returns blank. It doesn't work. The loop doesn't even get executed.

Answer 1

while(!in.eof()) is out of the question because eof doesn't work like that.

Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?

I'm not sure I see the problem, what's wrong with

size_t i = 0;
while (in >> frames[i])
    ++i;

(apart from the possibility of array bounds errors).

EDIT

This code is incorrect

 friend std::istream& operator>>(std::istream& in, XYZ_frame_read &traj) {
     in >> traj.num_particles;
     in >> traj.curr_frame;
     Particle p;
     while(in >> p){
          traj.coordinates_t.push_back(p);
     }
     return in;
 }

This says keep reading particles until a read fails. That's incorrect, you know how many particles there are. It should say keep reading particles until you have read num_particles of them (or a read fails). I.e. it should say

 friend std::istream& operator>>(std::istream& in, XYZ_frame_read &traj) {
     in >> traj.num_particles;
     in >> traj.curr_frame;
     Particle p;
     for (int i = 0; i < traj.num_particles && in >> p; ++i) 
          traj.coordinates_t.push_back(p);
     }
     return in;
 }

Answer 2

You are very close, you simply need to validate your input within your overloaded operator functions, and don't use new!, instead just use a std::vector<XYZ_frame_read> frames;

For example with your overload of istream for class XYZ_frame_read all you need is:

    friend std::istream& operator>>(std::istream& in, XYZ_frame_read &traj)
    {
        /* validate that num_particles and curr_frame read */
        if (in >> traj.num_particles >> traj.curr_frame) {
            int n = traj.num_particles; /* set number of particles to read */
            Particle p;

            while (n-- && (in >> p))    /* read that number of particles */
                traj.coordinates_t.push_back(p);
        }
        return in;
    }

Then in main() instead of allocating with new for frames as you do here:

    XYZ_frame_read* frames = new XYZ_frame_read[3];
    in >> frames[0];
    std::cout << frames[0];

just use a std::vector<XYZ_frame_read> frames; and then use a temporary class XYZ_frame_read to validate the read before adding it to the vector of frames, e.g.

    std::vector<XYZ_frame_read> frames; /* vector of frames (NO new!) */

    for (;;) {                      /* continual loop while good input */
        XYZ_frame_read tmp;         /* temp XYZ_frame_read for read */

        if ((in >> tmp))            /* if read is good */
            frames.push_back(tmp);  /* add it to vector of frames */
        else
            break;                  /* otherwise -- bail */
    }

For output, just use an auto-ranged for loop, e.g.

    for (auto & f : frames)         /* auto-ranged for loop to output frames */
        std::cout << "\nframe: " << f.get_frame() << 
                    "  particles: " << f.get_nparticles() << "\n\n" << 
                    f << '\n';

Putting it altogether, you would have:

#include <iostream>
#include <vector>
#include <fstream>

struct Particle {

    long double x,y,z;
    char tab ='\t';
    char newline = '\n';
    char atom ;

    friend std::istream& operator>>(std::istream& in, Particle &xyz) {
        in >> xyz.atom >> xyz.x >> xyz.y >> xyz.z;
        return in;
    }

    friend std::ostream& operator<<(std::ostream& out, Particle &xyz) {
        out << xyz.x << xyz.tab << xyz.y << xyz.tab << xyz.z << xyz.newline;
        return out;
    }
};

class XYZ_frame_read
{
    int curr_frame;
    int num_particles;
    std::vector<Particle> coordinates_t;

  public:

    friend std::istream& operator>>(std::istream& in, XYZ_frame_read &traj)
    {
        /* validate that num_particles and curr_frame read */
        if (in >> traj.num_particles >> traj.curr_frame) {
            int n = traj.num_particles; /* set number of particles to read */
            Particle p;

            while (n-- && (in >> p))    /* read that number of particles */
                traj.coordinates_t.push_back(p);
        }
        return in;
    }
    friend std::ostream& operator<<(std::ostream& out, XYZ_frame_read &traj) {

        for(int i = 0; i< traj.num_particles ;i ++)
            out << traj.coordinates_t.at(i) ;

        return out;
    }
    int get_frame(void) { return curr_frame; }
    int get_nparticles (void) { return num_particles; }
    int getpsize(void) { return coordinates_t.size(); }
};

int main(int argc, char *argv[]) {

    std::ifstream in(argv[1]);
    std::vector<XYZ_frame_read> frames; /* vector of frames (NO new!) */

    for (;;) {                      /* continual loop while good input */
        XYZ_frame_read tmp;         /* temp XYZ_frame_read for read */

        if ((in >> tmp))            /* if read is good */
            frames.push_back(tmp);  /* add it to vector of frames */
        else
            break;                  /* otherwise -- bail */
    }

    for (auto & f : frames)         /* auto-ranged for loop to output frames */
        std::cout << "\nframe: " << f.get_frame() << 
                    "  particles: " << f.get_nparticles() << "\n\n" << 
                    f << '\n';

    return 0;
    (void)argc;     /* suppress -Wunused warning */
}

There are huge number of advantages to using vector for class XYZ_frame_read instead of allocating with new. Auto memory-management is just the tip of the iceberg.

Example Input File

Using your sample input:

$ cat particles.txt
5
0
C    1.23    2.33    4.56
C    1.23    2.33    5.56
C    1.23    2.33    6.56
C    1.23    2.33    7.56
C    1.23    2.33    8.56
5
1
C    2.23    2.33    4.56
C    2.23    3.33    5.56
C    2.23    4.33    6.56
C    2.23    5.33    7.56
C    2.23    6.33    8.56

Example Use/Output

Just provide the filename and your std::vector<XYZ_frame_read> frames is automatically filled, regardless how many frames are present in your data file (up to the limits of your virtual-memory)

$ ./bin/paticle particles.txt

frame: 0  particles: 5

1.23    2.33    4.56
1.23    2.33    5.56
1.23    2.33    6.56
1.23    2.33    7.56
1.23    2.33    8.56


frame: 1  particles: 5

2.23    2.33    4.56
2.23    3.33    5.56
2.23    4.33    6.56
2.23    5.33    7.56
2.23    6.33    8.56