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 inmain()
, 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 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 tostd::cerr
, and then returnEXIT_FAILURE
:std::ifstream infile("File_1.prn"); if (!infile) { std::cerr << "error reading file"; return EXIT_FAILURE; }
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 inmain()
, 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 tostd::cerr
, and then returnEXIT_FAILURE
:std::ifstream infile("File_1.prn"); if (!infile) { std::cerr << "error reading file"; return EXIT_FAILURE; }
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 inmain()
, 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 tostd::cerr
, and then returnEXIT_FAILURE
:std::ifstream infile("File_1.prn"); if (!infile) { std::cerr << "error reading file"; return EXIT_FAILURE; }
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 inmain()
, 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 tostd::cerr
, and then returnEXIT_FAILURE
:std::ifstream infile("File_1.prn"); if (!infile) { std::cerr << "error reading file"; return EXIT_FAILURE; }