3
\$\begingroup\$

This is a follow up question to Date Checking in C++

I have these two function prototypes:

void CovidDB::add_covid_data(std::string const COVID_FILE)

void CovidDB::add_rehashed_data(std::string const COVID_FILE)

both essentially do the same thing except that one re_hashes the data before logging it to a .log file. I've heard about function pointers but was never really comfortable using them. Is there an STL container or something that would help me in this case?

Is the stuff I am doing with defining "random" macros such as LOG or WAIT bad practice?

I would like to know if there is anything that needs significant improvement, and if there is any other way of documenting code. One of my professors introduced me to doxygen but I found that it takes me a lot longer than just

// {
// desc:
// pre:
// post:
// }

I would also be very grateful if someone could point me on how to log errors without creating a logging class. Is there a library for that?

Edit: I am very familiar with Python, but I was forced to start using C++ in Data Structures (class I am taking right now), that's why I added the beginner tag.

Here is the full code:

run.h

#include <string>
#include "CovidDB.h"

/** DOCUMENTATION:
 * @author Jakob Balkovec
 * @file run.cpp [Source Code]
 * @note Header file for helper functions
 * @remark this file defines a set of helper functions that most get or check parameters 
 */

#pragma once

/** @brief 
 * @name get_date: Takes a string reference as an argument and returns a string 
 * @name get_country: Takes a string reference as an argument and returns a string.
 * @name get_country: Takes a string reference as an argument and returns a string.
 * @name get_coivd_cases: Takes an integer reference as an argument and returns an integer.
 * @name get_covid_deaths: Takes an integer reference as an argument and returns an integer.
 */
std::string get_date();
std::string get_country();
int get_coivd_cases();
int get_covid_deaths();

/** @brief 
 * @name set_data: Takes a DataEntry* pointer and several arguments
 * @param (country, date, cases, deaths) 
 * to set the data.
 */
void set_data(DataEntry* data, std::string country, std::string date, int cases, int deaths);

/** @brief 
 * @name get_input: Takes no arguments and returns an integer.
 * @name goodbye: Takes no arguments and returns void.
 * @name menu: Takes no arguments and returns void.
 * @name run: Takes no arguments and returns void.
*/
int get_input();
void goodbye();
void menu();
void run();

/** @brief 
 * @name valid_month: Takes a string and returns a boolean indicating the validity of the month.
 * @name valid_day: Takes a string and returns a boolean indicating the validity of the day.
 * @name valid_year: Takes a string reference and returns a boolean indicating the validity of */
bool valid_month(std::string);
bool valid_day(std::string);
bool valid_year(std::string &year);

run.cpp

#include <string>
#include <iostream>
#include <vector>
#include <chrono>
#include <algorithm>
#include <limits>
#include <stdexcept>
#include <thread>
#include <atomic>

#include "run.h"
#include "CovidDB.h"

#define LOG

typedef std::numeric_limits<int> int_limits;
static int constexpr INT_MAX = int_limits::max();

/** DOCUMENTATION:
 * @author Jakob Balkovec
 * @file assignment4.cpp [Driver Code]
 * @note Driver code for A4
 * 
 * @brief This assigment focuses on using multiple operations regarding BST like:
 *   - Insertion
 *   - Traversals
 *   - Searching
 *   - Deletion
 * 
 * Those operations were implemented using a ShelterBST class that includes a struct for Pets and
 * A struct for the TreeNodes.  
 */


/** @todo
 * test and assert
 * Clarify what to print
 * Make the output neater
*/


/** @name OPID (opeartion ID)
 * @enum for the switch statement
 * @brief Every operation has a numerical value
 */
enum OPID {
           ID_1 = 1,
           ID_2 = 2,
           ID_3 = 3,
           ID_4 = 4,
           ID_5 = 5,
           ID_6 = 6,
           ID_7 = 7,
           ID_8 = 8,
           ID_9 = 9,
           ID_10 = 10
};

/**
 * @brief Displays the menu for user interaction.
 */
void menu() {
  std::cout << "\n*** Welcome to the COVID-19 Data Base ***\n\n"
            << "[1]\tCreate a Hash Table with the WHO file\n"
            << "[2]\tAdd a Data entry\n"
            << "[3]\tGet a Data entry\n"
            << "[4]\tRemove a Data entry\n"
            << "[5]\tDisplay HasH Table\n"
            << "[6]\tRehash Table\n"
            << "[7]\tLog Data [covid_db.log]\n"
            << "[8]\tLog Re-hashed Data [rehash_covid_db.log]\n"
            << "[9]\tClear screen\n"
            << "[10]\tQuit\n";
}

/**
 * @brief Takes a string and returns a boolean indicating the validity of the month.
 * 
 * @param month The input month as a string.
 * @return True if the month is valid, false otherwise.
 */
bool valid_month(std::string month) {
  if(month.length() != 1 and month.length() != 2) {
    return false;
  } else if (std::stoi(month) > 13) {
    return false;
  } else if (std::stoi(month) <  1) {
    return false;
  }
  return true;
}

/**
 * @brief Takes a string and returns a boolean indicating the validity of the day.
 * 
 * @param day The input day as a string.
 * @return True if the day is valid, false otherwise.
 */
bool valid_day(std::string day) {
  if(day.length() != 1 and day.length() != 2) {
    return false;
  } else if (std::stoi(day) > 31) {
    return false;
  } else if (std::stoi(day) <  1) {
    return false;
  }
  return true;
}

/**
 * @brief Takes a string reference and returns a boolean indicating the validity of the year.
 * 
 * @param year The input year as a string reference.
 * @return True if the year is valid, false otherwise.
 */
bool valid_year(std::string &year) {
    if(year.length() == 4) {
      year = year[2] + year [3];
      return true;
    } else if(year.length() != 2) {
      return false;
    }
    return true;
}

/**
 * @brief Takes no parameters and returns a string.
 * @return The processed date as a string.
 * IMPORTANT: @invariant user does not enter a word input
 */
std::string get_date() {
  std::string date = "\0";
  std::string month = "\0";
  std::string day = "\0";
  std::string year = "\0";

  bool is_valid = false;

  while (!is_valid) {
    std::cout << "[FORMAT: mm/dd/yy][Enter a Date]: ";
    std::getline(std::cin, date);
    std::size_t month_loc = date.find("/");
    std::string month_prev = date.substr(0, month_loc);

    if (month_prev[0] == '0') {
      month = month_prev[1]; // if preceding 0 -> trim
    } else {
      month = month_prev; // if single digit -> keep
    }

    std::string s_str = date.substr(month_loc + 1);
    std::size_t day_loc = s_str.find("/");
    std::string day_prev = s_str.substr(0, day_loc);

    if (day_prev[0] == '0') {
      day = day_prev[1];
    } else {
      day = day_prev;
    }

    year = s_str.substr(day_loc + 1);
    is_valid = valid_day(day) && valid_month(month) && valid_year(year);

    //{
    /** @brief
     * c_ stands for current
     * e_ satnds for entered
    */
    //}
    if (is_valid) {
      try {
        std::time_t now = std::time(nullptr);
        std::tm* c_time = std::localtime(&now);

        int c_day = c_time->tm_mday;
        int c_month = c_time->tm_mon + 1; // Month is zero-based
        int c_year = c_time->tm_year % 100; // Last two digits of the year

        const int e_day = std::stoi(day);
        const int e_month = std::stoi(month);
        const int e_year = std::stoi(year);

        if (e_year > c_year) {
          is_valid = false; // Date is in the future
          throw std::invalid_argument("\n[Invalid]\n");
        } else if (e_year == c_year) {
          if (e_month > c_month) {
            is_valid = false; // Date is in the future
            throw std::invalid_argument("\n[Invalid]\n");
          } else if (e_month == c_month) {
            if (e_day > c_day) {
              is_valid = false; // Date is in the future
              throw std::invalid_argument("\n[Invalid]\n");
            }
          }
        } else {
          return month + "/" + day + "/" + year;
        }
      } catch (const std::exception& error) {
        std::cout << error.what();
        is_valid = false;
      }
    }
  }
  return month + "/" + day + "/" + year;
}

/**
 * @brief Takes no arguments and returns a string.
 * @return The processed country as a string.
 */
std::string get_country() {
  std::string country;

  while (true) {
      std::cout << "[Enter a country]: ";
      std::cin >> country;
      std::cin.ignore();

      if (std::all_of(country.begin(), country.end(), [](char c) { return std::isalpha(c); })) {
          bool is_all_lowercase = std::all_of(country.begin(), country.end(), [](char c) { return std::islower(c); });

          if (is_all_lowercase) {
              country[0] = std::toupper(country[0]);
              std::transform(country.begin() + 1, country.end(), country.begin() + 1, [](char c) { return std::tolower(c); });
          }
          return country;
      } else {
          std::cout << "[Input must be a string]\n";
          country = ""; // Reset the input
      }
  }
}

/**
 * @brief Takes no parameters and returns an integer.
 * @return The processed Covid cases as an integer.
 */
int get_covid_cases() {
  int deaths;

  while (true) {
      std::cout << "[Enter cases]: ";
      std::cin >> deaths;

      if (deaths >= 0) {
          break;
      } else {
          std::cout << "[invalid input]\n";
          // clear the input stream and ignore any remaining characters
          std::cin.clear();
          std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
      }
  }
  return deaths;
}

/**
 * @brief Takes no parameters and returns an integer.
 * @return The processed Covid deaths as an integer.
 */
int get_covid_deaths() {
  int deaths;

  while (true) {
      std::cout << "[Enter deaths]: ";
      std::cin >> deaths;

      if (deaths >= 0) {
          break;
      } else {
          std::cout << "[invalid input]\n";
          // clear the input stream and ignore any remaining characters
          std::cin.clear();
          std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
      }
  }
  return deaths;
}

/**
 * @brief Prompts the user to enter a valid intiger coresponding to one of the valus in the menu
 *        the user is prompted to enter the input again if it's not a number
 *
 * @return The processed input as an integer.
 */
int get_input() {
  int const MIN = 1;
  int const MAX = 10;
  int choice = 0;
  std::cout << "\n[Enter]: ";
  
  while (true) {
    try {
      std::cin >> choice;
      if (std::cin.fail()) { //std::cin.fail() if the input is not an intiger returns true
        /// @link https://cplusplus.com/forum/beginner/2957/
        
        std::cin.clear(); // clear error flags
        std::cin.ignore(10000, '\n'); // ignore up to 10000 characters or until a newline is encountered
        throw std::invalid_argument("[Invalid input]");
      }
      else if (choice < MIN || choice > MAX) {
        throw std::out_of_range("[Input out of range. Please enter an integer between 1 and 8]");
      }
      else {
        return choice;
      }
    }
    catch (const std::exception& error) {
      std::cout << error.what() << std::endl;
      std::cout << "[Re-enter]: ";
    }
  }
}

/** @name goodbye()
 * @brief The function prompts the user goodbye
 * @remark Handles UI
 * @return void-type
 */
void goodbye() {
  std::cout << "\n\nGoodbye!\n\n";
}

/**
 * @brief clears screen
 */
static inline void clear_screen() {
  #define SLEEP std::this_thread::sleep_for(std::chrono::milliseconds(500))
  
  SLEEP;
  std::system("clear");
  SLEEP;
}

/**
 * @brief Takes a DataEntry* pointer and sets it's data
 * @param data A pointer to a DataEntry object.
 */
void set_data(DataEntry* data) {
  data->set_country(get_country());
  data->set_date(get_date());
  data->set_c_cases(get_covid_cases());
  data->set_c_deaths(get_covid_deaths());
}

/**
 * @brief Executes the main logic of the program in a while(true) loop.
 */
void run() {
  /** DECLARATIONS: */
  std::cout << std::endl << std::endl << std::flush;
  CovidDB data_base(17);
  DataEntry *data = new DataEntry();
  
  /** DECLARATIONS: */
  
  while(true) {
    menu();
    switch(get_input()) {
    case OPID::ID_1: {
      data_base.add_covid_data(COVID_FILE);
      break;
    }
      
    case OPID::ID_2: {
      set_data(data);
      bool added = data_base.add(data);
      if(added) {
        std::cout << "\n[Record added]\n";
      } else {
        std::cout << "\n[Failed to add the entry]\n";
      }
      break;
    }
      
    case OPID::ID_3: {
      data_base.fetch_data(data, get_country());
      break;
    }
      
    case OPID::ID_4: {
      data_base.remove(get_country());
      break;
    }
      
    case OPID::ID_5: {
      data_base.display_table();
      break;
    }
    
    case OPID::ID_6: {
      data_base.rehash();
      break;
    }
    
    case OPID::ID_7: {
      #ifdef LOG
      data_base.log();
      #else
      std::cout << "\n[Define [LOG macro in run.cpp] to run]\n";
      #endif // LOG
      break;
    }

    case OPID::ID_8: {
      #ifdef LOG
      data_base.log_rehashed();
      #else
      std::cout << "\n[Define [LOG macro in run.cpp] to run]\n";
      #endif // LOG      
      break;
    }
    case OPID::ID_9: {
      clear_screen();
      break;
    }

    case OPID::ID_10: {
      goodbye();
      delete data;
      std::exit(EXIT_SUCCESS);
      break;
    }

    default: {
      /** @note do nothing...*/
    }
    }
  }
  std::cout << std::endl << std::endl << std::flush;
}

main.cpp

#include <iostream>
#include "run.h"

/**
 * @brief 
 * @name show_last_compiled
 * @param file Name of the file that is being compiled
 * @param date Date of when the file was last compiled
 * @param time Time of when the file was last compiled
 * @param author Author of the code
 * @note all params are arrays of chars
 */
static inline void show_last_compiled(const char* file, const char* date, const char* time, const char* author) {
    std::cout << "\n[" << file << "] " << "Last compiled on [" << date << "] at [" << time << "] by [" << author << "]\n" << std::endl;
}

#define AUTHOR "Jakob" //define a macro

/**
 * @brief The entry point of the program.
 * @return [EXIT_SUCESS], the exit status of the program.
 */
int main() {
    show_last_compiled(__FILE__, __DATE__, __TIME__, AUTHOR);
    run();
    return EXIT_SUCCESS;
}

CovidDB.cpp

/** DOCUMENTATION:
 * @author Jakob Balkovec
 * @file CovidDB.cpp [Driver Code]
 * @note Driver code for A4
 * 
 * @brief This assigment focuses on using multiple operations regarding HasHTables such as:
 *   - Insertion
 *   - Printing
 *   - Hashing
 *   - Deletion
 *   - [File reading]
 * 
 * Those operations were implemented using a DataEntry and a CovidDB class
 */

#include <string>
#include <cstring>
#include <sstream>
#include <iostream>
#include <fstream>
#include <cassert>
#include <ctime>
#include <chrono>
#include <iomanip>
#include <thread>
#include <cmath>
#include <cstdlib>
#include <unistd.h>
#include <atomic>
#include <sys/types.h>
#include <sys/wait.h>
#include <algorithm>

#include "CovidDB.h"

#define LOG
//#define WAIT //-> for submmission uncomment

/**
 * @brief Constructs an object of DataEntry type with default parameters
 * @return DataEntry Object
 */
DataEntry::DataEntry() {
  this->date = "\0";
  this->country = "\0";
  this->c_cases = 0;
  this->c_deaths = 0;
}

/**
 * @brief Hashfunction that creates a hash
 * @param country std::string entry -> country in the CSV file
 * @return Hash
 */
int CovidDB::hash(std::string country) {
  int sum = 0;
  int count = 0;
  
  for (char c : country) {
    sum = sum + ((count + 1) * c);
    count++;
  }
  return sum % size; //returns the hash
}

int CovidDB::re_hash_key(std::string country) {
  int sum = 0;
  int count = 0;
  
  for (char c : country) {
    sum = sum + ((count + 1) * c);
    count++;
  }
  return sum % size; //returns the new hash
}

/**
 * @brief Re-Hashfunction that rehashes the whole table
 */
void CovidDB::rehash() {
  auto is_prime = [](int num) {
    if (num <= 1)
      return false;

    for (int i = 2; i <= std::sqrt(num); ++i) {
      if (num % i == 0)
        return false;
    }
    return true;
  };

  auto find_next_prime = [&is_prime](int num) {
    while (true) {
      if (is_prime(num))
        return num;
      ++num;
    }
  };

  int new_size = size * 2;
  int new_prime_size = find_next_prime(new_size);

  // Create a new hash table with the new size
  CovidDB new_table(new_prime_size);

  // Rehash all entries from the original table to the new table
  for (std::vector<DataEntry*>& row : HashTable) {
    for (DataEntry* entry : row) {
      if (entry != nullptr) {
        int re_hash_i = re_hash_key(entry->get_country());
        new_table.HashTable[re_hash_i].push_back(entry);
      }
    }
  }
  for(auto row : HashTable) {
    row.clear();
  }
  HashTable.clear();

  HashTable = std::move(new_table.HashTable);
  size = new_prime_size;

  #ifdef LOG
  std::cout << "[LOG]: Table rehashed. New table size: " << size << std::endl;
  #endif //LOG
  return;
}

/**
 * @brief Inserts one data entry into the hash table. 
 * @param entry The DataEntry object to be added
 * @return true if record is added, false if record is rejected (date < than current date)
 */
bool CovidDB::add(DataEntry* entry) {
  time_t now = time(0);
  tm* ltm = localtime(&now);
  // DATE FORMAT: mm/dd/yy
  std::string current_date_str = std::to_string(1 + ltm->tm_mon) + "/" + std::to_string(ltm->tm_mday) + "/" + std::to_string(ltm->tm_year % 100);
  std::istringstream iss(current_date_str);
  std::tm current_date = {};
  iss >> std::get_time(&current_date, "%m/%d/%y");
  
  std::tm entry_date = {};
  std::istringstream iss2(entry -> get_date());
  iss2 >> std::get_time(&entry_date, "%m/%d/%y");
  
  if (mktime(&current_date) > mktime(&entry_date)) {
    std::cout << "[Record rejected]" << std::endl;   
    return false;
  }
  
  int index = hash(entry -> get_country());
  
  if (HashTable[index].empty()) {
    HashTable[index].push_back((entry));
  } else {
    bool added = false;
    for (DataEntry* existing_entry : HashTable[index]) {
      std::atomic<bool> valid(false);
      valid.store(hash(existing_entry->get_country()) == hash(entry->get_country()) &&
                       existing_entry->get_country() == entry->get_country());
      if (valid) {
        existing_entry->set_date(entry -> get_date());
        existing_entry->set_c_cases(existing_entry->get_c_cases() + entry->get_c_cases());
        existing_entry->set_c_deaths(existing_entry->get_c_deaths() + entry->get_c_deaths());
        added = true;
        //delete entry;
        break;
      }
    }
    if (!added) {
      HashTable[index].push_back(entry);
    }
  }
  return true;
}

/**
 * @brief Retrieves a data entry with the specific country name
 * @param country The country to search for
 * @return The DataEntry object with the matching country name, or NULL if no such entry exists
 */
DataEntry* CovidDB::get(std::string country) {
  int index = hash(country);
  assert(index < 17);
  
  for (DataEntry* entry : HashTable[index]) {
    if (entry->get_country() == country) {
      return entry;
    }
  }
  return nullptr;
}

/**
 * @brief Fetches the data entry for a specific country and assigns it
 *        to the provided DataEntry pointer.
 * 
 * @param set A pointer to a DataEntry object where the fetched data will be assigned.
 * @param country The name of the country to fetch data for.
 * @return void
 */
void CovidDB::fetch_data(DataEntry* set, std::string country) {
  set = get(country);
  if(set == nullptr) {
    std::cout << "\n[No entry found for: \"" << country << "\"]\n";
    return; //if nullptr don't derefernece
  }
  std::cout << set << std::endl;
}

/**
 * @brief Removes the data entry with the specific country name
 * @param country The country to remove
 */
void CovidDB::remove(std::string country) {
  int index = hash(country);
  
  for (auto it = HashTable[index].begin(); it != HashTable[index].end(); ++it) {
    if ((*it)->get_country() == country) {
      delete *it;
      HashTable[index].erase(it);
      return;
    }
  }
  std::cout << "\n[No entry found for: " << country << "]" << std::endl;
}

/**
 * @brief Prints the contents of the hash table using
 *        nested for each loops
*/
//{
// @bug when adding 2 entires with the same hash -> SIGSEV 
//}
void CovidDB::display_table() const {
  char const STICK = '|';
  bool is_empty = true;

  /** @note guard against printing an empty table*/
  for(const auto& vec : HashTable) { //if 1st dimension is empty
    if(!vec.empty()) {
      is_empty = false;
      break;
    }
  }

  if(is_empty) {
    std::cout << "\n[Data Base is empty]\n";
    return;
  }
  /** @note guard against printing an empty table*/
  
  std::cout << "\n[Printing Data Base]\n|\n";
  for (int i = 0; i < 3; i++) {
    std::this_thread::sleep_for(std::chrono::milliseconds(200));
    std::cout << STICK << std::endl;
  }
  std::cout << std::flush; //flush buffer
  std::string const SPACE = " ";
  
  for(std::vector<DataEntry*> vec : HashTable) {
    for(DataEntry* entry : vec) {
      if (entry != nullptr) { //guard against dereferencing nullptr
        std::cout << std::flush;
        std::cout << entry << std::endl;
      }
    }
  }
  std::cout << std::endl;
  return;
}

/**
 * @brief Logs the contents of the hash table using
 *        nested for each loops and writes them to a .log file
 */
void CovidDB::log() {
  #ifdef LOG
  add_covid_data(COVID_FILE); //add data and log
  std::ofstream covid_file;
  covid_file.open("covid_db.log");

  if (covid_file.is_open()) {
    covid_file << std::flush;

    std::string const SPACE = " ";
    covid_file << "\n\n****************************** COIVD DATA LOG ******************************\n\n\n";
    for (auto& vec : HashTable) {
      for (auto& entry : vec) {
        if (entry != nullptr) {
          covid_file << "[Date: " << entry->get_date() << "]," << SPACE
                     << "[Country: " << entry->get_country() << "]," << SPACE
                     << "[Cases: " << entry->get_c_cases() << "]," << SPACE
                     << "[Deaths: " << entry->get_c_deaths() << "]"
                     << std::endl;
        }
      }
    }
  covid_file.close();
  } else {
    covid_file << "\n[Error opening the file covidDB.log]\n";
    std::exit(EXIT_FAILURE);
  }
  std::this_thread::sleep_for(std::chrono::milliseconds(500));
  std::cout << "\n------ [Log avalible] ------\n\n";
  std::this_thread::sleep_for(std::chrono::milliseconds(500));
  std::exit(EXIT_SUCCESS);
  return;
}
#else
std::cout << "\n[Define [LOG macro in CovidDB.cpp] to run]\n";
#endif // LOG

/**
 * @brief Logs re-hashed data
 */
void CovidDB::log_rehashed() {
  #ifdef LOG
  add_covid_data(COVID_FILE); //add data and log
  std::ofstream covid_file;
  covid_file.open("rehash_covid_db.log");

  if (covid_file.is_open()) {
    covid_file << std::flush;

    std::string const SPACE = " ";
    covid_file << "\n\n************************** REHASHED COIVD DATA LOG **************************\n\n\n";
    for (auto& vec : HashTable) {
      for (auto& entry : vec) {
        if (entry != nullptr) {
          //use of overloaded << ostream operator
          covid_file << entry << std::endl;
        }
      }
    }
  covid_file.close();
  } else {
    covid_file << "\n[Error opening the file rehash_covidDB.log]\n";
    std::exit(EXIT_FAILURE);
  }
  std::this_thread::sleep_for(std::chrono::milliseconds(500));
  std::cout << "\n------ [Rehash Log avalible] ------\n\n";
  std::this_thread::sleep_for(std::chrono::milliseconds(500));
  std::exit(EXIT_SUCCESS);
  return;
}
#else
std::cout << "\n[Define [LOG macro in CovidDB.cpp] to run]\n";
#endif // LOG

/**
 * @brief Reads a CSV file containing COVID data and adds the data to the database.
 * @param COVID_FILE The name of the CSV file to read.
 * @return void
 */
void CovidDB::add_covid_data(std::string const COVID_FILE) {
  std::ifstream file(COVID_FILE);

  // Measure time it takes to fetch and process data
  std::chrono::steady_clock::time_point start_time;
  std::chrono::steady_clock::time_point end_time;

  start_time = std::chrono::steady_clock::now(); // Start stopwatch

  if (!file) {
    std::cout << "\n[File ERROR]\n " << COVID_FILE << std::endl;
    std::exit(EXIT_FAILURE);
  }

  std::cout << "\n[Fetching Data]\n";

  std::string line;
  std::getline(file, line); // Skip header line

  std::string latest_date_str = "11/02/22"; // Initialize to an old date
  std::tm latest_date = {};
  std::istringstream iss(latest_date_str);
  iss >> std::get_time(&latest_date, "%m/%d/%y");

  // Get the total number of lines in the file
  std::ifstream count_lines(COVID_FILE);
  int total_lines = std::count(std::istreambuf_iterator<char>(count_lines), std::istreambuf_iterator<char>(), '\n');
  count_lines.close();

  int progress_interval = total_lines / 10; // Update progress every 10% of the total lines
  int current_line = 0;

  // Progress bar variables
  int progress_bar_width = 40;

  while (std::getline(file, line)) {
    std::stringstream ss(line);
    std::string country, date_str, cases_str, deaths_str;
    std::getline(ss, date_str, ',');
    std::getline(ss, country, ',');
    std::getline(ss, cases_str, ',');
    std::getline(ss, deaths_str, ',');

    int cases = std::stoi(cases_str);
    int deaths = std::stoi(deaths_str);

    std::tm entry_date = {};
    std::istringstream iss2(date_str);
    iss2 >> std::get_time(&entry_date, "%m/%d/%y");

    if (mktime(&entry_date) > mktime(&latest_date)) {
      latest_date_str = date_str;
      latest_date = entry_date;
    }

    DataEntry* entry = new DataEntry();
    entry->set_country(country);
    entry->set_date(latest_date_str);
    entry->set_c_cases(cases);
    entry->set_c_deaths(deaths);

    add(entry);

    current_line++;

    // Update progress bar
    if (current_line % progress_interval == 0 || current_line == total_lines) {
      int progress = static_cast<int>((static_cast<double>(current_line) / total_lines) * 100);

      std::cout << "\r[";
      int completed_width = progress_bar_width * progress / 100;
      for (int i = 0; i < progress_bar_width; ++i) {
        if (i < completed_width) {
          std::cout << "#";
        } else {
          std::cout << " ";
        }
      }
      std::cout << "] [" << progress << "%]" << std::flush;
    }
  }
  file.close();

  end_time = std::chrono::steady_clock::now(); // Stop stopwatch

  std::chrono::duration<double> elapsedSeconds = end_time - start_time;

  // Static cast elapsed time to an int and round up
  auto elapsedSecondsCount = static_cast<int>(std::round(elapsedSeconds.count()));
  std::cout << "\n[Data Fetched] [Elapsed Time: " << elapsedSecondsCount << "s]\n\n";
  std::this_thread::sleep_for(std::chrono::milliseconds(1000));
  return;
}

/**
 * @brief Reads a CSV file containing COVID data and adds the new data to the database.
 * @param COVID_FILE The name of the CSV file to read.
 * @return void
 */
void CovidDB::add_rehashed_data(std::string const COVID_FILE) {
    std::ifstream file(COVID_FILE);

  // Measure time it takes to fetch and process data
  std::chrono::steady_clock::time_point start_time;
  std::chrono::steady_clock::time_point end_time;

  start_time = std::chrono::steady_clock::now(); // Start stopwatch

  if (!file) {
    std::cout << "\n[File ERROR]\n " << COVID_FILE << std::endl;
    std::exit(EXIT_FAILURE);
  }

  std::cout << "\n[Fetching Data]\n";

  std::string line;
  std::getline(file, line); // Skip header line

  std::string latest_date_str = "11/02/22"; // Initialize to an old date
  std::tm latest_date = {};
  std::istringstream iss(latest_date_str);
  iss >> std::get_time(&latest_date, "%m/%d/%y");

  // Get the total number of lines in the file
  std::ifstream count_lines(COVID_FILE);
  int total_lines = std::count(std::istreambuf_iterator<char>(count_lines), std::istreambuf_iterator<char>(), '\n');
  count_lines.close();

  int progress_interval = total_lines / 10; // Update progress every 10% of the total lines
  int current_line = 0;

  // Progress bar variables
  int progress_bar_width = 40;

  while (std::getline(file, line)) {
    std::stringstream ss(line);
    std::string country, date_str, cases_str, deaths_str;
    std::getline(ss, date_str, ',');
    std::getline(ss, country, ',');
    std::getline(ss, cases_str, ',');
    std::getline(ss, deaths_str, ',');

    int cases = std::stoi(cases_str);
    int deaths = std::stoi(deaths_str);

    std::tm entry_date = {};
    std::istringstream iss2(date_str);
    iss2 >> std::get_time(&entry_date, "%m/%d/%y");

    if (mktime(&entry_date) > mktime(&latest_date)) {
      latest_date_str = date_str;
      latest_date = entry_date;
    }

    DataEntry* entry = new DataEntry();
    entry->set_country(country);
    entry->set_date(latest_date_str);
    entry->set_c_cases(cases);
    entry->set_c_deaths(deaths);

    add(entry);
    rehash();

    current_line++;

    // Update progress bar
    if (current_line % progress_interval == 0 || current_line == total_lines) {
      int progress = static_cast<int>((static_cast<double>(current_line) / total_lines) * 100);

      std::cout << "\r[";
      int completed_width = progress_bar_width * progress / 100;
      for (int i = 0; i < progress_bar_width; ++i) {
        if (i < completed_width) {
          std::cout << "#";
        } else {
          std::cout << " ";
        }
      }
      std::cout << "] [" << progress << "%]" << std::flush;
    }
  }
  file.close();

  end_time = std::chrono::steady_clock::now(); // Stop stopwatch

  std::chrono::duration<double> elapsedSeconds = end_time - start_time;

  // Static cast elapsed time to an int and round up
  auto elapsedSecondsCount = static_cast<int>(std::round(elapsedSeconds.count()));
  std::cout << "\n[Data Fetched] [Elapsed Time: " << elapsedSecondsCount << "s]\n\n";
  std::this_thread::sleep_for(std::chrono::milliseconds(1000));
  return;
}

CovidDB.h

#include <iostream>
#include <string>
#include <vector>
#include <utility>

#pragma once

static std::string const COVID_FILE = "WHO-COVID-data.csv";

/** DOCUMENTATION:
 * @author Jakob Balkovec
 * @file CovidDB.h [Header file]
 * @note Header file for DataEntry and CovidDB class
 * 
 * @brief This assigment focuses on using multiple operations regarding HasHTables such as:
 *   - Insertion
 *   - Printing
 *   - Hashing
 *   - Deletion
 *   - [File reading]
 * 
 * Those operations were implemented using a DataEntry and a CovidDB class
 */


/**
 * @class DataEntry
 * @brief DataEntry class represents a single entry of COVID-19 data 
 *        for a particular date and country.
 * @note This class is immutable once created.
 */
class DataEntry final {
private:
  std::string date;
  std::string country;
  int c_cases;
  int c_deaths;
  
public:
  DataEntry();
  
  /** @note mutators and acessors*/
  
  inline void set_date(std::string set_date) { this->date = set_date;};
  inline void set_country(std::string set_country) { this->country = set_country;};
  inline void set_c_deaths(int set_c_deaths) { this->c_deaths = set_c_deaths;};
  inline void set_c_cases(int set_c_cases) { this->c_cases = set_c_cases;};
  
  inline int get_c_cases() { return c_cases;};
  inline int get_c_deaths() { return c_deaths;};
  
  inline std::string get_date() { return date;};
  inline std::string get_country() { return country;};   

  inline static void print_data_entry(std::ostream& stream, DataEntry* entry) {
  char const SPACE = ' ';
  if (entry != nullptr) {
    stream << "[Date: " << entry->get_date() << "]," << SPACE
           << "[Country: " << entry->get_country() << "]," << SPACE
           << "[Cases: " << entry->get_c_cases() << "]," << SPACE
           << "[Deaths: " << entry->get_c_deaths() << "]";
    }
  }

  //declare as "friend" so it doesn't give a fuck about access specifiers
  inline friend std::ostream& operator<<(std::ostream& stream, DataEntry* entry) {
  print_data_entry(stream, entry);
  return stream;
}

};

/**
 * @brief A hash table implementation to store Covid-19 data by country
 * @class CovidDB
 * @note The hash table size is fixed at 17.
 */
class CovidDB final {
private:
  int size;
  std::vector<std::vector<DataEntry*>> HashTable;
  
public:
  //non default construcotr with parameters
  //@param size -> custom size
  inline CovidDB(int size) : size(size), HashTable(size) {};

  //default construcotr
  inline CovidDB() { //default size
    HashTable.resize(17);
  }
  
  inline void clear() {
    for (auto& row : HashTable) {
      for (auto& entry : row) {
        delete entry;
      }
      row.clear();
    }
    HashTable.clear();    
    HashTable.shrink_to_fit();
  }

  inline ~CovidDB() { //handles memory
    clear();
  }
  
  /** @note Copy constructor */
  CovidDB(const CovidDB& other) {
    size = other.size;
    HashTable.resize(size);
    for (int i = 0; i < size; ++i) {
      for (const auto& entry : other.HashTable[i]) {
        HashTable[i].push_back(new DataEntry(*entry));
      }
    }
  }

  /** @note Move constructor */
  CovidDB(CovidDB&& other) noexcept
      : size(other.size)
      , HashTable(std::move(other.HashTable))
  {
    other.size = 0;
  }

  /** @note Overloaded assigment operator*/
  CovidDB& operator=(CovidDB other) { 
    std::swap(other.size, size);
    std::swap(other.HashTable, HashTable); 
    return *this; 
  }

  DataEntry* get(std::string country);
  void fetch_data(DataEntry* set, std::string country);
  bool add(DataEntry* entry);
  void add_covid_data(std::string const COVID_FILE);
  void add_rehashed_data(std::string const COVID_FILE);
  void rehash();
  int re_hash_key(std::string country);
  int hash(std::string country);
  void remove(std::string country);
  void display_table() const; 
  void log();
  void log_rehashed();
}; 

MakeFile

CC = g++
CFLAGS = -Wall -Werror -std=c++11 -pedantic -O3

SRCS = run.cpp CovidDB.cpp main.cpp
OBJS = $(SRCS:.cpp=.o)

TARGET = main

all: $(TARGET)

$(TARGET): $(OBJS)
    $(CC) $(CFLAGS) $(OBJS) -o $(TARGET)

%.o: %.cpp
    $(CC) $(CFLAGS) -c $< -o $@

clean:
    rm -f $(OBJS) $(TARGET)
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

Avoid macros where possible

Is the stuff I am doing with defining "random" macros such as LOG or WAIT bad practice?

First of all, in C++ you almost always be able to avoid using macros to begin with. Instead, create static constexpr constants (these also work for strings) or write regular functions. So consider:

static constexpr bool log = true;
static constexpr const char* author = "Jakob";

static void sleep() {
    std::this_thread::sleep_for(std::chrono::milliseconds(500));
}

Instead of #ifdef, you can write this:

if constexpr (log) {
    data_base.log();
} else {
    std::cout << "\n[Set variable `log` in run.cpp to `true` to run]\n";
}

You can omit the constexpr in that if-statement, although then both the if and the else part must be valid code.

Use a logging library

I would also be very grateful if someone could point me on how to log errors without creating a logging class. Is there a library for that?

Logging can be more important than you think, and doing logging properly can be a lot of work. So instead of implementing it yourself, indeed consider using an existing library, for example spdlog or Boost::log, but there are many others.

About Doxygen

One of my professors introduced me to doxygen but I found that it takes me a lot longer than just // {desc: … pre: … post …}

The reason to use the Doxygen format is so that there are tools that understand your documentation. For example, Doxygen (the program) will be able to create nice hyperlinked HTML and PDF files. But there are also code editors that understand Doyxgen comments, and will be able to show you the documentation when you hover over a function name for example.

If you believe it takes too much time writing it, then try to find some way to make it more efficient to write Doyxgen comments. For example, most code editors allow you to create macros or store snippets that allow you to insert a template Doxygen comment, and then you only have to fill in the details. See also this StackOverflow question.

Use '\n' instead of std::endl

Use '\n' instead of std::endl; the latter is equivalent to the former, but will also force the output to be flushed, which is usually unnecessary and might hurt performance.

For example, in this line you are flushing the output thrice:

std::cout << std::endl << std::endl << std::flush;

Especially at the start of run(), what is the point of the flushing anyway? Nothing else was printed at this point. On most operating systems, the standard output is line-buffered, so there is no need to explicitly flush after a newline.

Avoid declaring functions no other source file is going to use

In run.h you are declaring a lot of functions. However, with the exception of run(), all of these functions are only ever used inside run.c. So instead all the declarations could be removed from run.h, and the corresponding definitions in run.c be made static. This avoids polluting the global namespace with lots of generic function names.

Consider merging run.cpp into main.cpp

Your main() basically just calls run(), and main.cpp doesn't do much else. Consider just moving all of run.cpp into main.cpp; this avoids having to declare a globally visible function run().

Showing the timestamp of compilation

Another issue is show_last_compiled(). While technically what it prints is correct, it is actually not very useful. Because you have most of the code in other source files, it's in the other source files where you will make changes. So when you compile your project, your build system will likely not recompile main.cpp. What use is showing the date of compilation of just main.cpp? Unfortunately, there is no standard C++ solution to get the date when the whole binary was built, however your build system might be configured in a way to make this happen, or you might use some tricks like having a static object in each source file that registers its compilation time in its constructor, and then in main() just print the latest of those times.

Personally, I would rather avoid printing the build time; its use is limited to begin with, and even if you make it work, there are issues like whether your clock was set correctly.

Note that since C++20, you no longer have to use a macro to get the name of the file that is being compiled. Instead, you can use std::source_location. However, it doesn't come with a way to get the time of compilation.

Avoid fluff

As a beginning programmer, you might be tempted to add a personal tough or some flair to your program. While the intention might be good, try to avoid any fluff, like clearing the screen, sleeping unnecessarily, showing fake progress bars, and so on. These things don't add any functionality, might start to irritate you at some point, require more code that has to be written and can contain bugs, and make your program less efficient.

Take clear_screen(): every time it is called, you have to wait a second for no reason. It also uses std::system(), which forks a new shell process, which in turn parses the command you gave it, which in turn will fork a new process to execute /usr/bin/clear, which in turn is written in C itself and will just print an ANSI code to the screen to clear it. That's a huge overhead, and has even more issues: first, it is not portable (on Windows you'd have to use "cls" as the command), and second its behavior can change depending on your environment settings (consider that your $PATH might be such that it finds a clear that's not /usr/bin/clear).

While you can clear the screen by just printing an ANSI escape code, consider whether the screen needs to be cleared at all. If there is no good reason, just don't do it. The same goes for the unnecessary sleeps and the fake progress bar in display_table(). Consider:

void CovidDB::display_table() const {
  std::cout << "[Printing database]\n";
  bool empty = true;
  
  for(auto& vec : HashTable) {
    for(auto* entry : vec) {
      if (entry != nullptr) {
        empty = false;
        std::cout << entry << '\n';
      }
    }
  }

  if (empty) {
     std::cout << "[The database is empty]\n";
  }
}

Notice how much shorter that function is now! That's less code to write, less chance of bugs, and easier to maintain. And you get the data printed immediately instead of having to wait 600 milliseconds!

Spelling and profanity

There are a few spelling errors in your code, like "intiger", "COIVD", and the consistent "data base" instead of "database" (it's one word). This happens to everyone, even native speakers. Consider using a spell checker once in a while to find mistakes and correct them. A rather nice one to use on source code is codespell.

I also saw this:

//declare as "friend" so it doesn't give a fuck about access specifiers

Avoid using profanity in your code. Not everyone appreciates it, and if you ever have to collaborate with others, they might object to it. Also consider that if your code is public, then others can find it. Consider that if you apply for a job, the recruiters might search for code you wrote. It's best to just write neutral and boring comments.

Avoid manual memory allocation

Instead of using new and delete manually, prefer having memory managed by containers and smart pointers instead. In your case, you already have a container that stores entries, so you can have that manage your memory directly:

std::vector<std::vector<DataEntry>> HashTable;

This simplifies your code a lot. For example, you no longer need a destructor in class CovidDB. Since entries are stored by value, you also don't need a copy constructor to make deep copies of pointers.

Since there are no pointers anymore, there is also no possibility that there is a nullptr. So all those checks can go away.

One advantage of pointers was that you can pass them around without having to copy the whole object. To make adding new entries to the database still be efficient, some changes should be made. First, add a constructor to DataEntry:

class DataEntry final {
private:
  std::string date;
  std::string country;
  int c_cases = 0;
  int c_deaths = 0;
  
public:
  DataEntry() = default;
  DataEntry(const std::string& date, const std::string& country,
            int c_cases, int c_deaths):
     date(date), country(country), c_cases(c_cases), c_deaths(c_deaths) {}
   …
};

Then in CovidDB, take entry by value, but std::move() it into the table:

bool CovidDB::add(DataEntry entry) {
    …
    HashTable[index].push_back(std::move(entry));
    …
}

Then in add_covid_data() for example, then instead of allocating a new entry and calling the set_*() functions, you can write:

add({country, latest_date_str, cases, deaths});

This way, a temporary DataEntry object is created that is moved all the way into the table, without unnecessary copies. With whole-program optimization enabled, the compiler can probably even optimize out the moves.

Why implement your own hash table?

Instead of implementing your own hash table, you could have just used std::unordered_map.

If the goal of the excercise was to have you create your own hash table, then I would try to separate the hash table part from the COVID database; basically so that you write something that looks like:

template<typename T>
class MyHashTable {
    …
};

class CovidDB {
    MyHashTable<std::vector<DataEntry>> HashTable;
    …
};

Uninitialized value

If you use the default constructor of CovidDB, the member variable HashTable is initialized, but size is not.

However, instead of initializing size, I would just remove this member variable: you can just use HashTable.size() to get the size of the table. This avoids you having to keep your own size in sync.

Unnecessary use of final and inline

The keyword inline does nothing for member functions defined in the class declaration, those are already implicitly inline.

While marking a class as final might be useful in some cases, why do it here? There is no inheritance being used anywhere. And what if you really do want to inherit from CovidDB at some point? Would that really be a problem?

Avoid calling std::exit() from class member functions

By calling std::exit() from class member functions, you take away the possibility of the caller recovering from the error. Instead, consider throwing an exception instead (preferrably one of the standard exceptions or a custom class derived from those). If the exceptions are not caught, this will still cause the program to exit with a non-zero exit code, but now it allows the caller to try-catch and deal with the problem.

Missing error checking

You are doing a little bit of error checking on input/output, but there is a lot more that can go wrong than just opening the file: there could be a read or write error at any point for example.

When reading a file, when you think you have reached the end, check if file.eof() == true. If not, then an error occured.

When writing a file, check that file.good() == true after calling file.close(). If not, an error occured writing any part of it.

\$\endgroup\$
3
  • 2
    \$\begingroup\$ This seems to be a follow up question to date checker. The explanation for writing their own hash table is there, it is a requirement by the instructor, \$\endgroup\$
    – pacmaninbw
    May 29, 2023 at 13:42
  • \$\begingroup\$ std::endl "practice" was introduced to me in one of my early c++ classes, apparently;y it's supposed to make the output neater since it shifts it 2 lines down \$\endgroup\$ May 29, 2023 at 22:35
  • \$\begingroup\$ Sure, but I would then recommend doing std::cout << "\n\n". \$\endgroup\$
    – G. Sliepen
    May 30, 2023 at 9:28
1
\$\begingroup\$

General Observations

The review provided by G. Sliepen is excellent. They do point out some things I recommended in the first code review.

At first look I didn't see enough changes to justify another review so quickly, then I diffed all the files and there was more than 500 lines of changes, which does justify a new code review.

When your code gets reviewed, solve all the issues you are going to solve before continuing on to new development. Some of the code changes were new development.

In some professional code reviews the author of the code gets a numbered list of the review remarks. The author records the changes for each remark, or whether they will make a change or not, with a reason for the decision. All observations must be dealt with in some manner before new code can be developed.

Here on code review we look to see if the recommended changes have been made, there is no point in reviewing code again of the code hasn't improved.

Well Written and Designed Object Oriented Code

Maintainability

There are many things that go into writing good code, one is maintainability, which is writing code that is easy to read, so that it is easy to modify, this code with minor improvements is maintainable.

Re-usability

A second measure of writing good code is can that code be reused. This is part of what objected oriented code is about. Classes / objects should provide just enough functionality that they can be used in multiple programs if possible.

Portability

For code to be reusable it must be portable. Portable code is code that will compile and run on multiple operating systems. In C++17 portable file system code was added for just this reason.

This code appears to be written on Linux since it is using header files that are only available on Linux (unistd.h and sys/wait.h). These 2 header files are not available on Windows, at least when one is using Visual Studio 2022. I have removed the includes for these 2 files and the code still compiles, which means that these files are not actually necessary. For a number of reasons, only include header files that are necessary to make the code compile (mentioned in the previous review).

std::endl

The following line is in the void run() function twice, it is the first line of code in the function and the last line of code in the function.

    std::cout << std::endl << std::endl << std::flush;

That line of code is actually flushing the output 3 times. Each std::endl also flushes the output. If this code weren't part of the user interface, you might actually see a performance problem. This code would perform much better if it was written either as

    std::cout << "\n\n" << std::flush;

or

    std::cout << "\n" < std::endl;

It is actually very rare that the output needs to be flushed.

DRY Code

There is a programming principle called the Don't Repeat Yourself Principle sometimes referred to as DRY code. If you find yourself repeating the same code mutiple times it is better to encapsulate it in a function. If it is possible to loop through the code that can reduce repetition as well.

The following 2 functions in run.cpp are an example of Cut and Paste programming, which is generally a poor way to program. There are ways to combine these 2 functions into one function since they do the same thing.

/**
 * @brief Takes no parameters and returns an integer.
 * @return The processed Covid cases as an integer.
 */
int get_covid_cases() {
    int deaths;

    while (true) {
        std::cout << "[Enter cases]: ";
        std::cin >> deaths;

        if (deaths >= 0) {
            break;
        }
        else {
            std::cout << "[invalid input]\n";
            // clear the input stream and ignore any remaining characters
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        }
    }
    return deaths;
}

/**
 * @brief Takes no parameters and returns an integer.
 * @return The processed Covid deaths as an integer.
 */
int get_covid_deaths() {
    int deaths;

    while (true) {
        std::cout << "[Enter deaths]: ";
        std::cin >> deaths;

        if (deaths >= 0) {
            break;
        }
        else {
            std::cout << "[invalid input]\n";
            // clear the input stream and ignore any remaining characters
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        }
    }
    return deaths;
}

Since neither function is checking the input for > INT_MAX any more, this code at the top of the file is unnecessary and should be removed:

typedef std::numeric_limits<int> int_limits;
static int constexpr INT_MAX = int_limits::max();

As was noted in the previous review, INT_MAX is provided by #include <limits> and this code is attempting to redefine it.

Self Documenting Code

Comments must be maintained as well as the code, if the comments are out of date they can cause bugs to be introduced in the code. For the most part comments should be used to explain design and implementation decisions. Enums and Symbolic Constants are 2 ways to make the code self documenting.

Comments

The comment in the following code doesn't clarify anything, it is also incorrect since the first line following it is not a declaration.

    /** DECLARATIONS: */
    std::cout << std::endl << std::endl << std::flush;
    CovidDB data_base(17);
    DataEntry* data = new DataEntry();

Enums

These enums do not promote self documenting code:

enum OPID {
    ID_1 = 1,
    ID_2 = 2,
    ID_3 = 3,
    ID_4 = 4,
    ID_5 = 5,
    ID_6 = 6,
    ID_7 = 7,
    ID_8 = 8,
    ID_9 = 9,
    ID_10 = 10
};

A better set of enums would be:

enum OPID {
    CREAT_HASH_TAB,
    ADD_DATA_ENTRY,
    GET_DATA_ENTRY,
    REMOVE_DATA_ENTRY,
    DISPLAY_HASH_TAB,
    REHASH_HASH_TAB,
    LOG_DATA,
    LOG_REHASHED_DATA,
    CLEAR_SCREEN,
    QUIT
};

static const int MIN_OPID = static_cast<int>(OPID::CREAT_HASH_TAB);
static const int MAX_OPID = static_cast<int>(OPID::QUIT);

/**
 * @brief Displays the menu for user interaction.
 */
void menu() {
    std::cout << "\n*** Welcome to the COVID-19 Data Base ***\n\n"
        << "[" << static_cast<int>(OPID::CREAT_HASH_TAB) << "]\tCreate a Hash Table with the WHO file\n"
        << "[" << static_cast<int>(OPID::ADD_DATA_ENTRY) << "]\tAdd a Data entry\n"
        << "[" << static_cast<int>(OPID::GET_DATA_ENTRY) << "]\tGet a Data entry\n"
        << "[" << static_cast<int>(OPID::REMOVE_DATA_ENTRY) << "]\tRemove a Data entry\n"
        << "[" << static_cast<int>(OPID::DISPLAY_HASH_TAB) << "]\tDisplay HasH Table\n"
        << "[" << static_cast<int>(OPID::REHASH_HASH_TAB) << "]\tRehash Table\n"
        << "[" << static_cast<int>(OPID::LOG_DATA) << "]\tLog Data [covid_db.log]\n"
        << "[" << static_cast<int>(OPID::LOG_REHASHED_DATA) << "]\tLog Re-hashed Data [rehash_covid_db.log]\n"
        << "[" << static_cast<int>(OPID::CLEAR_SCREEN) << "]\tClear screen\n"
        << "[" << static_cast<int>(OPID::QUIT) << "]\tQuit\n";
}

int get_input() {
    int choice = 0;
    std::cout << "\n[Enter]: ";

    while (true) {
        try {
            std::cin >> choice;
            if (std::cin.fail()) { //std::cin.fail() if the input is not an intiger returns true
                /// @link https://cplusplus.com/forum/beginner/2957/

                std::cin.clear(); // clear error flags
                std::cin.ignore(10000, '\n'); // ignore up to 10000 characters or until a newline is encountered
                throw std::invalid_argument("[Invalid input]");
            }
            else if (choice < MIN_OPID || choice > MAX_OPID) {
                throw std::out_of_range("[Input out of range. Please enter an integer between 1 and 8]");
            }
            else {
                return choice;
            }
        }
        catch (const std::exception& error) {
            std::cout << error.what() << std::endl;
            std::cout << "[Re-enter]: ";
        }
    }
}

void run() {
    std::cout << "\n" << std::endl;
    CovidDB data_base(17);
    DataEntry* data = new DataEntry();

    while (true) {
        menu();
        switch (get_input()) {
        case OPID::CREAT_HASH_TAB: {
            data_base.add_covid_data(COVID_FILE);
            break;
        }

        case OPID::ADD_DATA_ENTRY: {
            set_data(data);
            bool added = data_base.add(data);
            if (added) {
                std::cout << "\n[Record added]\n";
            }
            else {
                std::cout << "\n[Failed to add the entry]\n";
            }
            break;
        }

        case OPID::GET_DATA_ENTRY: {
            data_base.fetch_data(data, get_country());
            break;
        }

        case OPID::REMOVE_DATA_ENTRY: {
            data_base.remove(get_country());
            break;
        }

        case OPID::DISPLAY_HASH_TAB: {
            data_base.display_table();
            break;
        }

        case OPID::REHASH_HASH_TAB: {
            data_base.rehash();
            break;
        }

        case OPID::LOG_DATA: {
            data_base.log();
            break;
        }

        case OPID::LOG_REHASHED_DATA: {
\            data_base.log_rehashed();
            break;
        }

        case OPID::CLEAR_SCREEN: {
            clear_screen();
            break;
        }

        case OPID::QUIT: {
            goodbye();
            delete data;
            std::exit(EXIT_SUCCESS);
            break;
        }

        default: {
            /** @note do nothing...*/
        }
        }
    }
    std::cout << "\n" << std::endl;
}

Please note, the enums do not need values assigned to them, which makes it more maintainable.

Contents of Header Files Can be a Liability

As G. Sliepen observed only put public interfaces into header files.

This function prototype in run.h

/** @brief
 * @name set_data: Takes a DataEntry* pointer and several arguments
 * @param (country, date, cases, deaths)
 * to set the data.
 */
void set_data(DataEntry* data, std::string country, std::string date, int cases, int deaths);

does not match the actual implementation in run.cpp:

/**
 * @brief Takes a DataEntry* pointer and sets it's data
 * @param data A pointer to a DataEntry object.
 */
void set_data(DataEntry* data) {
    data->set_country(get_country());
    data->set_date(get_date());
    data->set_c_cases(get_covid_cases());
    data->set_c_deaths(get_covid_deaths());
}

This might actually cause compile or link errors in some compilers on some operating systems.

Since set_data() is only called within run.cpp it might be good to make it a static function and remove the declaration from the header file.

\$\endgroup\$
3
  • \$\begingroup\$ One of my professors in my first C++ class said: "Print 2 endl twice to shift the output down and make it neater" Now I see that's a bad habit I picked up. Thanks for the review and the feedback! \$\endgroup\$ May 30, 2023 at 17:29
  • \$\begingroup\$ The prototype doesn't match because I was still working on my assignment and I guess -Werror and -pedantic didn't raise the unused variable flag \$\endgroup\$ May 30, 2023 at 17:31
  • \$\begingroup\$ You don't need that prototype since it isn't being called by an external file. \$\endgroup\$
    – pacmaninbw
    May 30, 2023 at 17:44

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.