6
\$\begingroup\$

This is one of my first projects in C++, and I would call it a program for storing, displaying data and prognosing numbers.

I use following compiler: g++ (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0

I will try to explain my code starting with the main method which is located at the bottom of the code. The main method in my program is like the main menu. You have four options when you start the program: Displaying data, inputting data, prognosing numbers and quit. The source code is at the bottom of this post.

1. Displaying Data

When you choose this option, your ouput is following (if the data file is not empty):

Do you want to make a prognose (P), input new data (I), show current data (D) or quit (Q)?: d
___________________
|Day|Infected|Dead|
|1  |2       |0   |
|2  |2       |0   |
|3  |3       |0   |
|4  |3       |0   |
|5  |9       |0   |
|6  |14      |0   |
|7  |18      |0   |
|8  |21      |0   |
|9  |29      |0   |
|10 |41      |0   |
|11 |55      |0   |
|12 |79      |0   |
|13 |104     |0   |
|14 |131     |0   |
|15 |182     |0   |
|16 |246     |0   |
|17 |302     |1   |
|18 |504     |1   |
|19 |655     |1   |
|20 |860     |1   |
|21 |1016    |3   |
|22 |1332    |3   |
|23 |1646    |4   |
|24 |2053    |6   |
|25 |2388    |6   |
Do you want to make a prognose (P), input new data (I), show current data (D) or quit (Q)?: 

When choosing this option, the function displayData() with the parameter file_name is called. The function has two parameters, but the second one, prognose_days is defaultly set to -1. This is because this function has two ways of executing. The first way is only displaying the current data (prognose_days = -1), and the second way is displaying the current data + prognosed numbers (prognose_days = n).

After setting some variables, the function first checks, if the data file contains data. Then it takes every line from the data file and splits it at the delimeter :. The splitting is done with the method splitString(). The splitted lines get saved in a vector called vector_list.

The next condition checks if only the current data should get displayed, or also prognosed numbers. When prognosed numbers also should get displayed, it adds those to the end of the vector_list.

The rest of the function is pretty complicated for others to read, I think. The point of the rest is to display the table with the data correctly and in a good looking way. Firstly the longest length of each column (day, infected, dead) gets stored into a variable with the function getLongestLength(). Then the table gets outputted with help of ternary operator, with the correct width etc.

2. Inputting Data

When you choose this option, your output is the following:

Do you want to make a prognose (P), input new data (I), show current data (D) or quit (Q)?: i
Day 26: Enter new data or quit (Q): 2500:7 
Day 27: Enter new data or quit (Q): q
Do you want to make a prognose (P), input new data (I), show current data (D) or quit (Q)?: 

This option gets the numbers you type in (e.g 2500:7 -> 2500 infections and 7 deaths) and writes them with the help of the function writeFile() into the data file.

3. Prognosing Numbers

When you choose this option your output is the following:

Do you want to make a prognose (P), input new data (I), show current data (D) or quit (Q)?: p
How many days to you want to prognose?: 3
______________________
|Day   |Infected|Dead|
|1     |2       |0   |
|2     |2       |0   |
|3     |3       |0   |
|4     |3       |0   |
|5     |9       |0   |
|6     |14      |0   |
|7     |18      |0   |
|8     |21      |0   |
|9     |29      |0   |
|10    |41      |0   |
|11    |55      |0   |
|12    |79      |0   |
|13    |104     |0   |
|14    |131     |0   |
|15    |182     |0   |
|16    |246     |0   |
|17    |302     |1   |
|18    |504     |1   |
|19    |655     |1   |
|20    |860     |1   |
|21    |1016    |3   |
|22    |1332    |3   |
|23    |1646    |4   |
|24    |2053    |6   |
|25    |2388    |6   |
|26    |2500    |7   |
|27 (P)|3409    |9   |
|28 (P)|4649    |12  |
|29 (P)|6339    |16  |
Infections: 6339
Deaths: 16
Do you want to make a prognose (P), input new data (I), show current data (D) or quit (Q)?: 

This option calculates a prognose, by calculating the average factor for the numbers. Ex.

1 Infected
3 Infected (x3)
8 Infected (x2.66)
9 Infected (x1.125)
Average Factor: (3+2.66+1.125) / 3 = 2,26

Prognose for two days:
9*2,26 = 20,34
20,34*2,26 = 45,96

I know mathematically, this is not the best approach, but that is not important now. The function does exactly what I calculated above and returns an array with the prognosed infections and deaths.

4. Quit

Quits the program.

My questions are:

  • Does my code have any major flaws or no-gos in programming or programming in C++?
  • Should have I used pointers and/or addresses in my code?
  • How is the general style of my programming?
  • Did you notice anything else?

coronavirus.cpp

#include <stdio.h>
#include <math.h>
#include <iostream>
#include <fstream>
#include <sstream>
#include <string>
#include <algorithm>
#include <vector>

using namespace std;

// DECLARES METHODS
vector<string> splitString(string s, char delim);
// DEFAULTLY PROGNOSE DAYS IS -1 WHICH MEANS IT ONLY DISPLAYS CURRENT DATA AND NOT PROGNOSE DATA
void displayData(string file_name, int prognose_days = -1);

// PUBLIC VARIABLE FOR FILENAME
string file_name = "corona_numbers.dat";

// CALCULATES PROGNOSE WITH FILE AND PROGNOSE DAYS
vector<int> calculatePrognose(double days, string file_name) {
    int zero_dead = 0, zero_infected = 0;
    // DECLARES INPUT FILESTREAM TO READ FILE
    ifstream file(file_name);
    // DECLARS VECTOR WHICH CONTAINS VECTORS OF SPLITTED FILE-LINES
    vector<vector<string>> vector_list;
    string line;
    // DECLARES VARIABLES
    double avg_infected_factor, avg_dead_factor, last_infected_count, last_dead_count, infected = 0, dead = 0;
    vector<int> empty_file = { -1, -1};
    // CHECKS IF FILE EXISTS/IS EMPTY
    if (file.peek() == ifstream::traits_type::eof()) {
        cout << "\033[1;31m[ERROR]\033[0m File is empty. Please input data!" << endl;
        return empty_file;
    }
    
    // GETS LINES OF FILE, SPLITS THEM AND PUTS THEM IN A VECTOR
    while(getline(file, line)) {
        vector<string> splitted_string = splitString(line, ':');
        vector_list.push_back(splitted_string);
    }
    
    //TODO: CHANGE CALCULATION OF AVGFACTOR WITH SORTING OUT EXTREMES
    
    // SETS FIRST NUMBER OF DATA TO LASTCOUNT
    last_infected_count = stoi(vector_list[0][1]);
    last_dead_count = stoi(vector_list[0][2]);
    
    avg_dead_factor, avg_infected_factor = 0;
    
    // SUMS AVEREAGES OF COUNTS
    for(int i = 1; i < vector_list.size(); i++) {
        
        // CHECK IF LAST NUMBER IS ZERO TO PREVENT A DIVISION THROUGH ZERO
        if (last_dead_count != 0) {
            avg_dead_factor += stoi(vector_list[i][2]) / last_dead_count;
            last_dead_count = stoi(vector_list[i][2]);
            
        } else {
            // INCREMENT COUNTER FOR TIMES ZERO PEOPLE WERE DEAD
            zero_dead++;
        }
        
        // CHECK IF LAST NUMBER IS ZERO TO PREVENT A DIVISION THROUGH ZERO
        if (last_infected_count != 0) {
            avg_infected_factor += stoi(vector_list[i][1]) / last_infected_count;
            last_infected_count = stoi(vector_list[i][1]);
            
        } else {
            // INCREMENT COUNTER FOR TIMES ZERO PEOPLE WERE INFECTED
            zero_infected++;
        }
        
        // SET LAST INFECTED COUNT TO CURRENT COUNT
        last_infected_count = stoi(vector_list[i][1]);
        last_dead_count = stoi(vector_list[i][2]);
    }
    
  
    // CALCULATES AVERAGES
    avg_infected_factor = avg_infected_factor / (vector_list.size() - 1 - zero_infected);
    avg_dead_factor = avg_dead_factor / (vector_list.size() - 1 - zero_dead);
    
    // CALCULATES PROGNOSES
    infected = stoi(vector_list[vector_list.size() - 1][1]) * pow(avg_infected_factor, days);
    dead = stoi(vector_list[vector_list.size() - 1][2]) * pow(avg_dead_factor, days);
    
    vector<int> prognose = { (int) infected, (int) dead };
    
    // DISPLAYS PROGNOSEDATA IN TABLE
    
    return prognose;
}

// SPLITS STRING AT DELIMETER AND RETURNS A VECTOR
vector<string> splitString(string s, char delim) {
    string string_token;
    vector<string> string_vector;

    for (int i = 0; i < s.length(); i++) {
        if (s[i] == delim) {
            string_vector.push_back(string_token);
            string_token = "";
        } else {
            string_token += s[i];
        }
    }
    return string_vector;
}

// WRITES TO FILE
void writeFile(string content, string file_name) {
    ofstream file;
    file.open(file_name, ios_base::out | ios_base::app);
    file << content;
    file.close();
}

// GET LONGEST NUMBER IN DATA TO DISPLAY TABLE CORRECTLY
int getLongestLength(vector<vector<string>> vector_list, int index) {
    int result;
    result = vector_list[0][index].length();
    for(vector<string> splitted_string: vector_list) {
        if (splitted_string[index].length() > result) {
            result = splitted_string[index].length();
        }
    }
    return result;
}

// DISPLAY PROGNOSE FOR EACH DAY IN TABLE
// DISPLAYS CURRENT DATA AS TABLE
void displayData(string file_name, int prognose_days) {
    int width_day, width_infected, width_dead, current_day;
    string day = "Day";
    string infected = "Infected";
    string dead = "Dead";
    ifstream file(file_name);
    string line;
    vector<vector<string>> vector_list;
    
    // CHECKS IF FILE EXISTS/IS EMPTY
    if (file.peek() == ifstream::traits_type::eof()) {
        cout << "\033[1;31m[ERROR]\033[0m File is empty. Please input data!" << endl;
        return;
    }
    
    // GETS LINES OF FILE, SPLITS THEM AND PUTS THEM IN A VECTOR
    while(getline(file, line)) {
        vector<string> splitted_string = splitString(line, ':');
        vector_list.push_back(splitted_string);
    }
    
    
    if (prognose_days != -1) {
        vector<string> prognose_vector;
        ifstream file(file_name);
        string line;
        int current_day;
        while(getline(file, line)) {
            vector<string> splitted_string = splitString(line, ':');
            // GETS LAST DAY AND CONTINUES WITH THE NEXT
            current_day = stoi(splitted_string[0]) + 1;
        }
       
        for(int i = 1; i <= prognose_days; i++) {
            // CREATE EXTRA STRING TO ADD COLOR
            
            
            prognose_vector.push_back(to_string(current_day) + " (P)");
            prognose_vector.push_back(to_string(calculatePrognose(i, file_name)[0]));
            prognose_vector.push_back(to_string(calculatePrognose(i, file_name)[1]));
            
            
            vector_list.push_back(prognose_vector);
            current_day++;
            prognose_vector.clear();
        }
    }
    
    file.close();
    
    // GET LONGEST LENGTH OF EACH COLUMN
    width_day = getLongestLength(vector_list, 0);
    width_infected = getLongestLength(vector_list, 1);
    width_dead = getLongestLength(vector_list, 2);
    
    // DISPLAYS SEPERATORS IN CORRECT COUNT WITH TERNARY OPERATORS
    cout << string(((width_day < day.length()) ? 0 : width_day - day.length()) 
    + ((width_infected < infected.length()) ? 0 : width_infected - infected.length())
    + ((width_dead < dead.length()) ? 0 : width_dead - dead.length())
    + day.length() + infected.length() + dead.length() + 4, '_') << endl;
    
    // DISPLAYS DESCRIPTION IN CORRECT COUNT WITH TERNARY OPERATORS
    cout <<
    "|" << "\033[1;32m" << day << "\033[0m" << string((width_day < day.length()) ? 0 : width_day - day.length(), ' ') <<
    "|" << "\033[1;33m" << infected << "\033[0m" << string((width_infected < infected.length()) ? 0 : width_infected - infected.length(), ' ') <<
    "|" << "\033[1;31m" << dead << "\033[0m" << string((width_dead < dead.length()) ? 0 : width_dead - dead.length(), ' ') << 
    "|" << endl;
    
    // DISPLAYS DATA IN CORRECT COUNT WITH TERNARY OPERATORS
    for(vector<string> splitted_string: vector_list) {
        cout << 
        "|" << splitted_string[0] << string((splitted_string[0].length() < day.length())
        ? ((width_day < day.length()) ? day.length() - splitted_string[0].length() 
        : width_day - splitted_string[0].length()) : width_day - splitted_string[0].length(), ' ') <<
        "|" << splitted_string[1] << string((splitted_string[1].length() < infected.length())
        ? ((width_infected < infected.length()) ? infected.length() - splitted_string[1].length() 
        : width_infected - splitted_string[1].length()) : width_infected - splitted_string[1].length(), ' ') << 
        "|" << splitted_string[2] << string((splitted_string[2].length() < dead.length())
        ? ((width_dead < dead.length()) ? dead.length() - splitted_string[2].length() 
        : width_dead - splitted_string[2].length()) : width_dead - splitted_string[2].length(), ' ') << 
        "|" << endl;
    }
}


int main(int argc, char **argv) {
    string use_choice, infected_and_dead;
    double prognose_days;
    
    // LOOP TO GET TO MAIN MENU WHEN LEAVING A SUB MENU
    while (true) {
        // GETS CHOICE
        cout << "Do you want to make a prognose (P), input new data (I), show current data (D) or quit (Q)?: ";
        cin >> use_choice;
        // TRANSFORMS CHOICE TO LOWERCASE TO IGNORE CASE
        transform(use_choice.begin(), use_choice.end(), use_choice.begin(), ::tolower);
        
        // PROGNOSE
        if (use_choice == "p") {
            cout << "How many days to you want to prognose?: ";
            cin >> prognose_days;
            // CALCULATES PROGNOSE OUT OF SUBMITTED DAYS
            vector<int> prognose = calculatePrognose(prognose_days, file_name);
            
            if (prognose[0] && prognose[1] == -1) {
                continue;
            } else {
                displayData(file_name, prognose_days);
                cout << "\033[1;33mInfections:\033[0m " << prognose[0] << endl
                << "\033[1;31mDeaths: \033[0m" << prognose[1] << endl;
            }
            
        // INPUT NEW DATA
        } else if (use_choice == "i") {
            ifstream file(file_name);
            string line;
            int current_day;
            // CHECKS IF FILE EXISTS/IS EMPTY AND START WITH DAY 1
            if (file.peek() == ifstream::traits_type::eof()) {
                current_day = 1;
            } else {
                while(getline(file, line)) {
                    vector<string> splitted_string = splitString(line, ':');
                    // GETS LAST DAY AND CONTINUES WITH THE NEXT
                    current_day = stoi(splitted_string[0]) + 1;
                }
            }
            file.close();
            while (true) {
                cout << "Day " << current_day << ": Enter new data or quit (Q): ";
                cin >> infected_and_dead;
                // TRANSFORMS CHOICE TO LOWERCASE TO IGNORE CASE
                transform(infected_and_dead.begin(), infected_and_dead.end(), infected_and_dead.begin(), ::tolower);
                if (infected_and_dead == "q") {
                    current_day++;
                    break;
                }
                // CONVERTS NUMBERS TO STRINGS
                infected_and_dead = to_string(current_day) + ":" + infected_and_dead + ":" + "\n";
                // CHECKS IF DATA IS CORRECTLY INPUTTED
                if (splitString(infected_and_dead, ':').size() < 3) {
                    cout << "\033[1;31m[ERROR]\033[0m Invalid Input! Please enter data in this form: 'infected:dead' " << endl;
                    continue;
                }
                // WRITES DATA TO FILE
                writeFile(infected_and_dead, file_name);
                current_day++;
            }
        // DISPLAY DATA
        } else if (use_choice == "d") {
            displayData(file_name);
        // QUIT
        } else if (use_choice == "q") {
            break;
        } else {
            // DISPLAYS ERROR MESSAGE WHEN WRONG CHOICE IS SUBMITTED
            cout << "\033[1;31m[ERROR]\033[0m Invalid Input" << endl;
        }
    }
    return 0;
}

corona_numbers.dat (example)

1:10:0:
2:25:2:
3:50:3:
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! Can you edit the question to also indicate the revision of the standard and the compiler options you are using? Thanks. By the way, you can take the tour and browse our FAQs to further familiarize yourself with our community. \$\endgroup\$
    – L. F.
    Commented Mar 22, 2020 at 0:37

2 Answers 2

3
\$\begingroup\$

Welcome to Code Review! Let's go through your code and try to improve it.

Comments

The first thing I notice about your code is that all comments are written in ALL CAPS. This not only makes the comments harder to read, but also distracts the reader of the code. I suggest changing them to lower case.

Some comments talk about aspects of the C++ language rather than the purpose of the code:

// DECLARES METHODS
// PUBLIC VARIABLE FOR FILENAME

etc. These comments are unnecessary to readers familiar with C++, so you can remove them once you feel comfortable with basic C++ constructs.

using namespace std;

Putting a using namespace std; at global level is considered bad practice. It pulls in all names from the std namespace, potentially introducing name clashes. See Why is using namespace std; considered bad practice? for more information. For relatively small projects like yours, this is not a big problem, but I recommend that you remove this line and get used to qualifying names from the standard library with std:: as soon as possible.

Declarations and definitions of functions

Instead of declaring functions first and put the definitions after, you can simply define functions after their dependencies. That is, you can define splitString before calculatePrognose to eliminate the declaration.

Pass-by-value vs pass-by-reference

When you are reading data, passing by value requires copying the content of the argument. This is very expensive for large types like vector<vector<string>>. Pass by const reference (const vector<vector<string>>&) instead. See How to pass objects to functions in C++? for more information.

Data

Right now, the way you keep track of data is to maintain a vector<vector<string>> in each function and pass it around. First, each record (the vector<string>) can be made a simple class:

using data_t = long;

struct Record {
    data_t day;
    data_t infected;
    data_t dead;
};

We can overload >> to support input:

// simple manipulator
// is >> eat_delim{':'} eats ':' delimiter
struct eat_delim {
    char delim;
};

std::istream& operator>>(std::istream& is, eat_delim manip)
{
    if (is.peek() == manip.delim) {
        is.get();
    } else {
        is.setstate(is.failbit);
    }
    return is;
}

std::istream& operator>>(std::istream& is, Record& rec)
{
    return is >> rec.day      >> eat_delim{':'}
              >> rec.infected >> eat_delim{':'}
              >> rec.dead     >> eat_delim{':'};
}

Then, we can make a dedicated class to perform operations on the data:

class DataProcessor {
    std::vector<Record> data;
    std::string filename;
public:
    DataProcessor(std::string file)
        : filename{std::move(file)}
    {
        std::ifstream in{filename};
        for (Record rec; in >> rec;) {
            data.push_back(rec);
        }
    }
    // ...
};

splitString

Here's roughly how I would write the splitString method with string methods:

// a:b:c: => [a, b, c]
auto split_string(std::string_view string, char delim)
{
    std::vector<std::string> result;
    for (std::size_t index; (index = string.find(delim)) != string.npos) {
        result.emplace_back(string, 0, index);
        string.remove_prefix(index + 1);
    }
    return result;
}

This split_string discards content after the last delimiter, just like your version does. Note that string_view is used to avoid copying and size_t is used to index the string instead of int.

writeFile

This function can be simplified if you make use of RAII — automatic opening/closing:

std::ofstream file{file_name, std::ios_base::app};
file << content;

Note that out is always set for ofstream.

Under our new design, this operation will be carried out when new data is added, so that the file syncs with our data.

getLongestLength

This function becomes much simpler with C++20 projections:

std::ranges::max(
    vector_list | std::views::transform([&](const auto& record) {
        return record[index].size();
    })
)

calculatePrognose

The size of this function makes it hard to understand, so let's try to break it down. We calculate the average factor first and determine the prognosis later:

// pointer to member of Record
using column_t = data_t Record::*;

class DataProcessor {
    // ...
public:
    // ...
    double average_factor(column_t column)
    {
        if (data.size() <= 1) {
            throw std::invalid_argument{"Insufficient samples"};
        }
        std::vector<data_t> past_data(data.size());
        std::transform(data.begin(), data.end(), past_data, column);
        std::adjacent_difference(past_data.begin(), past_data.end(),
                                 past_data.begin(), std::divides{});
        return std::accumulate(past_data.begin() + 1, past_data.end(), 0.0) / (past_data.size() - 1);
    }
    std::vector<Record> prognosis(data_t days)
    {
        auto infected_factor = average_factor(&Record::infected);
        auto dead_factor = average_factor(&Record::dead);

        std::vector<Record> result;
        for (Record rec = data.back(); days--;) {
            ++rec.day;
            rec.infected *= infected_factor;
            rec.dead *= dead_factor;
            result.push_back(rec);
        }
        return result;
    }
    // ...
};

displayData

Again, the sheer length of the function makes it virtually incomprehensible (the last part is especially unreadable), so I'm just trying to rewrite it. The printing of the data rows (as opposed to prognosis rows) can be extracted into a separate function:

class DataProcessor {
    // ...
public:
    // ...
    void print_data(std::ostream& os) const
    {
        std::array<int, 3> widths {
            // calculate widths
        };

        auto total_width = widths[0] + widths[1] + widths[2] + 4;
        os << std::string(total_width, '_');
        print_data_rows(os, widths);
        print_stats(os);
    }
    // print_prognosis is similar
    // ...
private:
    void print_data_rows(std::ostream& os, const std::array<int, 3>& widths) const
    {
        for (const auto& record : data) {
            os << '|'
               << std::setw(widths[0]) << record.day
               << '|'
               << std::setw(widths[1]) << record.infected
               << '|'
               << std::setw(widths[2]) << record.dead
               << '|';
        }
    }
    void print_prognosis_rows(std::ostream& os, const std::array<int, 3>& widths, data_t days) const
    {
        auto prog = prognosis(days);
        for (const auto& record : prog) {
            os << '|'
               << std::setw(widths[0]) << (std::to_string(record.day) + " (P)")
               << '|'
               << std::setw(widths[1]) << (std::to_string(record.day) + " (P)")
               << '|'
               << std::setw(widths[2]) << (std::to_string(record.day) + " (P)")
               << '|';
        }
    }
    void print_stats(std::ostream& os)
    {
        // print Infections: 6339 etc.
    }
    // ...
};
\$\endgroup\$
3
\$\begingroup\$

It would have been nice to have the data file so that we could test the code to provide an even better review.

Complexity

Almost All of the functions are too complex (do too much) and should be broken up into multiple functions. Multiple functions are easier to read, write, debug and maintain. A single function that performs a single operation is much easier to debug and write. Specifically these functions are too complex, vector<int> calculatePrognose(double days, string file_name), void displayData(string file_name, int prognose_days = -1) and int main(int argc, char **argv).

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.

The entire while(true) loop in main() should be a function. The entire block of code within

        } else if (use_choice == "i") {
            ...
        }

Would be a sub-function of the while(true) loop function.

Another example, this snippet from vector calculatePrognose(double days, string file_name) would make a good function:

    for(int i = 1; i < vector_list.size(); i++) {

        // CHECK IF LAST NUMBER IS ZERO TO PREVENT A DIVISION THROUGH ZERO
        if (last_dead_count != 0) {
            avg_dead_factor += stoi(vector_list[i][2]) / last_dead_count;
            last_dead_count = stoi(vector_list[i][2]);

        } else {
            // INCREMENT COUNTER FOR TIMES ZERO PEOPLE WERE DEAD
            zero_dead++;
        }

        // CHECK IF LAST NUMBER IS ZERO TO PREVENT A DIVISION THROUGH ZERO
        if (last_infected_count != 0) {
            avg_infected_factor += stoi(vector_list[i][1]) / last_infected_count;
            last_infected_count = stoi(vector_list[i][1]);

        } else {
            // INCREMENT COUNTER FOR TIMES ZERO PEOPLE WERE INFECTED
            zero_infected++;
        }

        // SET LAST INFECTED COUNT TO CURRENT COUNT
        last_infected_count = stoi(vector_list[i][1]);
        last_dead_count = stoi(vector_list[i][2]);
    }

Avoid using namespace std;

If you are coding professionally you probably should get out of the habit of using the using namespace std; statement. The code will more clearly define where cout and other identifiers are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The identifiercout you may override within your own classes, and you may override the operator << in your own classes as well. This [stack overflow question][3] discusses this in more detail.

Initialize The Local Variables When They are Declared

In C++ local variables do not have a default value, they should all be initialized. My complier reports one warning where a variable is used before it is initialized, avg_dead_factor in vector<int> calculatePrognose(double days, string file_name).

uninitialized local variable 'avg_dead_factor' used

The code contains this line:

avg_dead_factor, avg_infected_factor = 0;

Only avg_infected_factor will be initialized in the above code.

To make it easier to maintain the code, each variable should be delcared and initialized on a single line:

vector<int> calculatePrognose(double days, string file_name) {
    // DECLARES VARIABLES
    int zero_dead = 0;
    int zero_infected = 0;
    double avg_infected_factor = 0;
    double avg_dead_factor = 0;
    double last_infected_count = 0;
    double last_dead_count = 0;
    double infected = 0;
    double dead = 0;

It is important to note that variables should be declared as they are needed and only within the scope that they are needed. The above snippet is only an example.

Code should be as self documenting as possible and comments like // DECLARES VARIABLES are not necessary.

Global Variables

Generally you've done a pretty good job of avoiding global variables, with one glaring exception:

string file_name = "corona_numbers.dat";

It might be better if this was declared in main() and passed to the functions that need it. This value could be an argument that is passed into the program from the command line to make the program easier to use without modifying it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.