HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Looping over files in C++ and changing their names to store data into a single file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
loopingfileintostorenamesfilessingleandchangingdata

Problem

I have a lot of data in separate files generated from a simulation program. All data is clear and formatted via a single space to read in 4 columns. I can open and store data from each file one at a time, but that will get so impossible once I hit more than, say, 10 files of data with the same simulation data columns.

I am just trying to find a better way to open files and store the data using an iterative method instead of typing, for example, "open file, read file data, append to file" over 1000 times.

#include 
#include 
#include 
#include 

using namespace std;

int main ()
{
// INITIALIZE
    int tmax = 80000;

    float time_step[tmax];

    float LEx[tmax];
    float LEy[tmax];
    float LEz[tmax];
    float LE[tmax];

    float AvgLEx;
    float AvgLEy;
    float AvgLEz;
    float AvgLE;

    float Function[tmax];

// FILES
    ifstream infile;
    infile.open("File_1.prn");
// THIS INFILE is where I want to put a loop to change File_1 to File_2 to File_3 and so on and so on to append data from all those files into ONE single file
    if(infile.fail())
    {
        cout > time_step[i] >> LEx[i] >> LEy[i] >> LEz[i] >> LE[i];
        AvgLEx = AvgLEx + LEx[i];
        AvgLEy = AvgLEy + LEy[i];
        AvgLEz = AvgLEz + LEz[i];
        AvgLE = AvgLE + LE[i];

    }

    AvgLEx = AvgLEx/tmax;
    AvgLEy = AvgLEy/tmax;
    AvgLEz = AvgLEz/tmax;
    AvgLE = AvgLE/tmax;

    cout  max)
            {
                max = A_dt;
            }

// OUTPUT
            ofstream AutoCor;
            AutoCor.open ("AutoCor_25.prn", ios::app);
            AutoCor << dt << " " << A_dt << "\n";
            AutoCor.close();
            cout << "A_dt: " << A_dt << " " << "window used: " << window << "\n";

        }

        cout << "max used: " << max << "\n";

// RETURN       
    return A_dt;
    return 0;

}

Solution

As you're new to C++, you should be aware that implementing an entire program in main() can make things difficult, especially when the code is already complex.

As it appears, you have declare and initialize variables at the top, open the file, perform the operations, then output things. Even with these comments in place, you should still split this up into functions.

-
Do not maintain a list of variables as it could make maintenance more difficult, such as if variables are no longer being used. Instead. you should only declare or initialize variables as close to their intended use as possible. That way, if you no longer use that variable, you won't have to search for it at the top. This will also make your code easier to follow.

-
Consider just having main() do two things: attempt to open the file and call all the necessary functions afterwards. You can also open the file in an other function, but if you do it in main(), then you can easily return from there if the file cannot be opened.

-
Everything else, especially the file operations, should be in separate functions. Also keep in mind that a function should just have one primary purpose, in order to maintain the Single Responsible Principle (SRP). If a function needs to do something else that another function already does, then it should call that function. You should also name functions, in the verb form, based on what they're supposed to do. This will make the function's intent very clear to others.

Example:

void openFile() {}


Miscellaneous:

-
Try not to use using namespace std in global scope as it could cause name-clashing bugs, especially in larger programs. See this post for more info on this.

-
This variable appears to be a constant:

int tmax = 80000;


If so, initialize it with const so that it cannot be modified:

const int tmax = 80000;


-
For this approach at checking the file:

infile.open("File_1.prn");

if(infile.fail())
{
    cout << "error reading file" << endl;
    return 1;
}


you should use std::ifstream, check for !infile(), print the error message to std::cerr, and then return EXIT_FAILURE:

std::ifstream infile("File_1.prn");

if (!infile)
{
    std::cerr << "error reading file";
    return EXIT_FAILURE;
}

Code Snippets

void openFile() {}
int tmax = 80000;
const int tmax = 80000;
infile.open("File_1.prn");

if(infile.fail())
{
    cout << "error reading file" << endl;
    return 1;
}
std::ifstream infile("File_1.prn");

if (!infile)
{
    std::cerr << "error reading file";
    return EXIT_FAILURE;
}

Context

StackExchange Code Review Q#57025, answer score: 2

Revisions (0)

No revisions yet.