7
\$\begingroup\$

I am working on one of my assignments (Hash Tables) for my Data Structures class(1st year), the assignment focuses on OOP and understanding of Hash Tables.

Restrictions:

  • Usage of raw pointers
  • C++11 standard
  • Have to use vectors for my HashTable (can't use std::map)

The goal of this assignment is to make a Covid-19 table, which is capable of reading data from a .csv file and filling the table with those. Besides that, there is supposed to be a UI that allows the user to enter entries manually.

Is this a good (low-level) OOP design? I am not familiar with Polymorphism, Inheritance, and so on just yet. Style tips are always welcome.

run.cpp

#include <string>
#include <iostream>
#include <vector>
#include <sstream>
#include <fstream>
#include <chrono>
#include <ctime>
#include <iomanip>
#include <algorithm>
#include <limits>
#include <thread>
#include <stdexcept>
#include <cctype>
#include <cassert>
#include <atomic>

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

#define LOG

/** DOCUMENTATION:
 * @author Jakob Balkovec
 * @file assignment3.cpp [Driver Code]
 * @note Driver code for A3
 * 
 * @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
};

/**
 * @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" //segfault
            << "[3]\tGet a Data entry\n"
            << "[4]\tRemove a Data entry\n" //segfault if no entry
            << "[5]\tDisplay HasH Table\n"
            << "[6]\tLog Data [covid_db.log]\n"
            << "[7]\tLog Memory [valgrind.log]\n"
            << "[8]\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 a string reference as an argument and returns a string.
 *
 * @param date The input date as a string reference.
 * @return The processed date as a string.
 * IMPORTANT: @invariant user does not enter a word input
 */

//{
  /** @bug FIX: future dates weren't getting rejected */
//}
std::string get_date(std::string &date) {
  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 a string reference as an argument and returns a string.
 *
 * @param country The input country as a string reference.
 * @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();

    try {
      //{
        //Using lambda expression to check is the input is not numeric
      //}
      if (!std::all_of(country.begin(), country.end(), [](char c){ return std::isalpha(c); })) {
        throw std::invalid_argument("[Input must be a string]\n");
      }
      std::atomic<bool> is_all_lowercase(false);  
      is_all_lowercase.store(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); });
      } else  {
        return country;
      }
    } catch (const std::exception& e) {
      std::cerr << "\nError: " << e.what() << '\n';
      country = ""; // Reset the input
    }
  }
}

/**
 * @brief Takes an integer reference as an argument and returns an integer.
 *
 * @param cases The input Covid cases as an integer reference.
 * @return The processed Covid cases as an integer.
 */
int get_covid_cases(int &cases) {
  typedef std::numeric_limits<int> IntLimits;
  
  const int INT_MAX = IntLimits::max();
  
  std::cout << "[Enter cases]: ";
  std::cin >> cases;
  
  try {
    if(cases < 0) {
      throw std::invalid_argument("[Cases cannot be negative!]");
    }
    if(cases > INT_MAX) {
        throw std::invalid_argument("[Integer overflow!]");
    } else {
      return cases;
    }
    
  } catch (const std::exception& e) {
    std::cerr << "\nError: " << e.what() << std::endl;
    return 0;
  }
  return 0;
}

/**
 * @brief Takes an integer reference as an argument and returns an integer.
 *
 * @param deaths The input Covid deaths as an integer reference.
 * @return The processed Covid deaths as an integer.
 */
int get_covid_deaths(int &deaths) {
  typedef std::numeric_limits<int> IntLimits;
  
  const int INT_MAX = IntLimits::max();
  
  while(true) {
    std::cout << "[Enter deaths]: ";
    std::cin >> deaths;
    try {
      if(deaths < 0) {
        throw std::invalid_argument("[Cases cannot be negative!]");
      }
      if(deaths > INT_MAX) {
        throw std::invalid_argument("[Integer overflow!]");
      } else {
        return deaths;
      }
    } catch (const std::exception& e) {
      std::cerr << "\nError: " << e.what() << std::endl;
      return 0;
    }
  }
  return 0;
}

/**
 * @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 = 8;
  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 Takes a DataEntry* pointer and several arguments (country, date, cases, deaths)
 *        to set the data.
 * 
 * @param data A pointer to a DataEntry object.
 * @param country The country associated with the data.
 * @param date The date associated with the data.
 * @param cases The number of Covid cases associated with the data.
 * @param deaths The number of Covid deaths associated with the data.
 */
void set_data(DataEntry* data, std::string country, std::string date, int cases, int deaths) {
  data->set_country(get_country(country));
  data->set_date(get_date(date));
  data->set_c_cases(get_covid_cases(cases));
  data->set_c_deaths(get_covid_deaths(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;
  DataEntry *data = new DataEntry; //#2 allocation (new DataEntry)
  
  std::string country = "\n";
  std::string date = "\0";
  int cases = 0;
  int deaths = 0;
  /** 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, country, date, cases, deaths);
      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(country));
      break;
    }
      
    case OPID::ID_4: {
      data_base.remove(get_country(country));
      break;
    }
      
    case OPID::ID_5: {
      data_base.display_table();
      break;
    }
      
    case OPID::ID_6: {
      #ifdef LOG
      data_base.log();
      #else
      std::cout << "\n[Define [LOG macro in run.cpp] to run]\n";
      #endif // LOG
      break;
    }

    case OPID::ID_7: {
      #ifdef LOG
      data_base.log_memory();
      std::exit(EXIT_SUCCESS);
      #else
      std::cout << "\n[Define [LOG macro in run.cpp] to run]\n";
      #endif
      break;
    }

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

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

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 &date);
std::string get_country(std::string &country);
int get_coivd_cases(int &cases);
int get_covid_deaths(int &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);

/**
 * @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
 */
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;
}

main.cpp

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

#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;
}

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)

CovidDB.h

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

#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;};   
};

/**
 * @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 = 17;
  std::vector<std::vector<DataEntry*>> HashTable;
  
public:
  inline CovidDB() {
    HashTable.resize(size);
  }
  
  inline void clear() {
    for (auto& row : HashTable) {
      for (auto& entry : row) {
        delete entry;
      }
      row.clear();
    }
    HashTable.clear();    
    HashTable.shrink_to_fit();
    ///@link https://stackoverflow.com/questions/12587774/destructor-not-being-called
  }

  inline ~CovidDB() { //handles memory
    clear();
    std::cout << "\nDESTROYED\n";
  }
  
  /** @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 { //noexcept is necessray -> doesn't throw errors
    size = other.size;
    HashTable = std::move(other.HashTable);
    other.size = 0;
  }

  /** @note Overloaded assigment operator*/
  CovidDB& operator=(const CovidDB& other) {
  if (this == &other) {
    return *this;  // Self-assignment, no work needed
  }
  
  // clear the data and resources
  for (auto& row : HashTable) {
    for (auto& entry : row) {
      delete entry;
    }
    row.clear();
  }
  HashTable.clear();
  HashTable.shrink_to_fit();

  // copy the data from the other object
  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));
    }
  }
  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);
  int hash(std::string country);
  void remove(std::string country);
  void display_table(); 
  void log();
  void log_memory();
}; 

CovidDB.h

/** 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 "CovidDB.h"

#define LOG
//#define WAIT

/**
 * @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
}

/**
 * @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());
  assert(index < 17 and index >= 0);
  
  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
  }
  char const SPACE = ' ';
  std::cout << std::flush;
  std::cout << "\n[Date: " << set -> get_date() << "]," << SPACE
            << "[Country: " << set -> get_country() << "]," << SPACE
            << "[Cases: " << set -> get_c_cases() << "]," <<SPACE
            << "[Deaths: " << set -> get_c_deaths() << "]" << SPACE
            << std::endl;
            std::this_thread::sleep_for(std::chrono::milliseconds(100));
}

/**
 * @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() {
  char const STICK = '|';
  bool is_empty = true;

  /** @note guard against printing an empty table*/
  for(const auto& vec : HashTable) { //if 1D 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 << "[Date: " << entry -> get_date() << "]," << SPACE
                  << "[Country: " << entry -> get_country() << "]," << SPACE
                  << "[Cases: " << entry -> get_c_cases() << "]," << SPACE
                  << "[Deaths: " << entry -> get_c_deaths() << "]"
                  << std::endl;
                  #ifdef WAIT
                  std::this_thread::sleep_for(std::chrono::milliseconds(100));
                  #endif //WAIT
      }
    }
  }
  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 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);

  /** @note measure timne it takes to fetch and process data*/
  std::chrono::steady_clock::time_point startTime;
  std::chrono::steady_clock::time_point endTime;

  startTime = 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 << std::flush;
  std::cout << "\n[Fetching Data]\n";
  std::cout << std::flush;
  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");
  
  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(); //#1 allocation
    entry->set_country(country);
    entry->set_date(latest_date_str);
    entry->set_c_cases(cases);
    entry->set_c_deaths(deaths);
    
    add(entry);
  }
  file.close();

  endTime = std::chrono::steady_clock::now(); //stop stopwatch

  std::chrono::duration<double> elapsedSeconds = endTime - startTime;

  /** @note static cast it to an int and round up*/
  auto elapsedSecondsCount = static_cast<int>(std::round(elapsedSeconds.count()));

  std::cout << std::flush; //flush isotream buffer
  std::cout << "|\n|\n|\n[Data Fetched] [Elapsed Time: " << elapsedSecondsCount << "s]\n" << std::flush;
  return;
} 

/**
 * @brief logs memory by running valgrind
 */
void CovidDB::log_memory() {
  #ifdef LOG
  std::cout << "\n[NOTE]: Please quit the program right after it re-runs with \"Valgrind\"\n";
  std::this_thread::sleep_for(std::chrono::seconds(4));
  std::string command = "valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes --num-callers=20 -s --log-file=valgrind.log ./main";
  std::system(command.c_str());
}
#else
std::cout << "\n[Define [LOG macro in CovidDB.cpp] to run]\n";
#endif
```
\$\endgroup\$
10
  • \$\begingroup\$ Please show more of the file, such as the includes at the top, and it would be very helpful to see how the get_date() function is being called. \$\endgroup\$
    – pacmaninbw
    May 27, 2023 at 19:16
  • 1
    \$\begingroup\$ 200 to 300 lines of code is not bad. We can handle up to 64K characters in a question. \$\endgroup\$
    – pacmaninbw
    May 27, 2023 at 19:27
  • 4
    \$\begingroup\$ @TheCompile-TimeComedian minimal examples are a StackOverflow thing, not a Code Review thing—but if you can isolate the part of the code you want feedback on into a self-contained unit, that’s great. Many good questions consist of one function and perhaps a test harness. \$\endgroup\$
    – Davislor
    May 27, 2023 at 20:56
  • 2
    \$\begingroup\$ Also, we encourage askers not to change the code once they’ve gotten answers about it, since that would make the answer out of date. \$\endgroup\$
    – Davislor
    May 27, 2023 at 21:18
  • 1
    \$\begingroup\$ Personally, I thought your original question was fine. \$\endgroup\$
    – Davislor
    May 27, 2023 at 21:19

2 Answers 2

8
\$\begingroup\$

Don't ignore the 2 most significant digits of the year, you are re-introducing the Y2K bug. The year can be off by a full century but the code will accept the date.

The get_date() function is way to complex (does too much). The helper functions that you have are about the right size, add some more helper functions. Test the users input in a function, only return from that function when the input is valid.

Rather than using std::endl output lines should be terminated with "\n" for performance reasons. Using std::endl means that you are flushing the output buffer after the output. This is generally not necessary, it also adds a system call to the output.

Rather than call std::exit() in the run() function it is better to return to the main() function. std::exit() should only be used in error cases where there is no possibility of recovery from the error. When writing operating system code always avoid calling std::exit(), it really isn't good to shut down the system.

The enum names could be improved, names like OPID::ID_8: really don't provide self documenting code.

Updated Review 5/28/2023

I have left the previous review comments because they are still valid. The following is based on the updated question with the additional code added.

General Observations

As far as Object Oriented Programming goes, you should learn SOLID.

SOLID is 5 object oriented design principles. SOLID is a mnemonic acronym for five design principles intended to make software designs more understandable, flexible and maintainable. This will help you design your objects and classes better.

  1. The Single Responsibility Principle - A class should only have a single responsibility, that is, only changes to one part of the software's specification should be able to affect the specification of the class.
  2. The Open–closed Principle - states software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.
  3. The Liskov Substitution Principle - Objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.
  4. The Interface segregation principle - states that no client should be forced to depend on methods it does not use.
  5. The Dependency Inversion Principle - is a specific form of decoupling software modules. When following this principle, the conventional dependency relationships established from high-level, policy-setting modules to low-level, dependency modules are reversed, thus rendering high-level modules independent of the low-level module implementation details.

You might want to implement the hash table as a separate class with it's own header and C file.

Based on the current code, it is questionable whether this program actually works as intended. If it does not then the question is off-topic for Code Review.

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 multiple 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 code is repeated in 2 functions, int get_covid_cases() and int get_covid_deaths():

    typedef std::numeric_limits<int> IntLimits;

    const int INT_MAX = IntLimits::max();

Rather than defining INT_MAX as a constant in 2 different functions define it as a constant for the entire file:

static constexpr int COVID_INT_MAX { std::numeric_limits<int>::max() };

I changed the name of INT_MAX to COVID_INT_MAX to avoid an issue with my compiler (Visual Studio 2022). INT_MAX is already defined in my compiler.

Possible Logic Issues in run.cpp

Both int get_covid_cases(int &) and int get_covid_deaths(int &) have some other common issues:

  • Both functions return a value in two ways, one in a return value and one in a parameter passed by reference.
  • It isn't clear that either function needs the try{} catch(){} block.
  • The prompt in both functions is missing the type of input that is needed (integer).

The while (true) loop in int get_covid_deaths() is not working as expected. The loop will always exit on the first iteration whether the value is correct or not.

In the case of the return methodology pick one, either return a value from the function or alter the parameter value but not both. The code that is calling these functions expects a return value, which is the choice you should make. This is the calling code:

void set_data(DataEntry* data, std::string country, std::string date, int cases, int deaths) {
    data->set_country(get_country(country));
    data->set_date(get_date(date));
    data->set_c_cases(get_covid_cases(cases));
    data->set_c_deaths(get_covid_deaths(deaths));
}

The set_data() function in it's current implementation does not need the arguments for country, date, cases or deaths because values of the arguments are never used.

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());
}

Magic Numbers

There are Magic Numbers in the function bool CovidDB::add(DataEntry* entry) function (0 and 17), it might be better to create symbolic constants for them to make the code more readble and easier to maintain. These numbers may be used in many places and being able to change them by editing only one line makes maintainence easier.

Numeric constants in code are sometimes referred to as Magic Numbers, because there is no obvious meaning for them. There is a discussion of this on stackoverflow.

Assert Statements

The assert() macro is a great debugging tool, but it is only a debugging tool, assert statements are not implemented in the resulting code when debugging is turned off (if the -g flag isn't used).

Inline Function Declarations

In the early days of C++ programming declaring a function as inline was a way to optimize code, this is no longer the case. The C++ compiler uses inline only as a recommendation and may ignore it. The -O2 and -O3 optimization flags produce more optimized code than a programmer generally can. Inline functions can only be inlined if they fit into the cache memory.

Only Include What Is Necessary

The include mechanism used in C++ actually includes the code from the include file, this can lead to increased compile / build times. It is better for many reasons to only include what is necessary to compile.

I have altered run.cpp, run.h and main.cpp to only include what is necessary:

main.cpp

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

static 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;
}

run.h

#pragma once

void run();

** Top of run.cpp **

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

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

#define LOG

Please note there was a lot of code reduction.

\$\endgroup\$
8
  • \$\begingroup\$ Thanks, I'll edit the question and provide more code \$\endgroup\$ May 27, 2023 at 20:38
  • \$\begingroup\$ I hope this helps, Thaks again \$\endgroup\$ May 27, 2023 at 20:46
  • \$\begingroup\$ Eh, the C++ standard library consistently uses snake_case. I’d say it’s pretty common. \$\endgroup\$
    – Davislor
    May 28, 2023 at 21:16
  • \$\begingroup\$ @Davislor you have a point. There were a whole lot of other issues that were more important. \$\endgroup\$
    – pacmaninbw
    May 28, 2023 at 21:23
  • 1
    \$\begingroup\$ Also, INT_MAX is defined in the standard library (<limits.h> or <climits>), so it really needs to change regardless. \$\endgroup\$
    – Davislor
    May 28, 2023 at 23:28
7
\$\begingroup\$

To focus on the specific thing you asked about:

You can use the > operator to do the comparison. You might compare two std::chrono::year_month_day objects, or two std::chrono::time_point objects. Either way, you could use std::chrono::from_stream to read the time from cin, std::chrono::utc_clock::now (which won’t have weird bugs around daylight savings time) to get the current time, and convert both into a common format that lets you compare if (input_time > current_time).

Currently, you’re returning string values and then checking a separate set of functions like valid_day. You could make your code much shorter and sweeter by using exceptions: if any step of the algorithm fails, throw an exception right away (maybe std::runtime_error) and catch it back in your input loop. This catch block can do things like print the error message it received. Then, your functions only return normally when they succeed, so you don’t need a lot of if statements to check for errors later—if there had been an error, control would have gone back to the catch block.

If you need to work with invalid results and not just short-circuit, having everything that could fail return a std::optional is a good approach. It’s harder to mess up than the traditional way of returning magic numbers, and makes it easier to compose and chain the steps of the computation than returning a success/error code and setting the result as a side-effect.

Edits

Two things jump out at me in the supplementary code: the enum with eight values from ID_1 = 1 to ID_8 = 8 is completely pointless. More importantly, don’t put function bodies in a .h file, since those will get copy-pasted into multiple source files. That will make compilation fail, because the linker will find conflicting definitions of the same function in multiple object files. (The exceptions are inline/constexpr/consteval functions, which are safe to put in headers but should be kept short and sweet. Those get expanded wherever they’re called, not turned into calls to a function that should only be compiled once, so they don’t need to have one and only one definition.)

\$\endgroup\$
7
  • \$\begingroup\$ Thanks, I'll get on that. I just read the documentation for std::optional and it was introduced in C++ 17, well I am bounded to the -std=c++11 compilation flag since those are the requirements for this class. \$\endgroup\$ May 27, 2023 at 20:13
  • \$\begingroup\$ @TheCompile-TimeComedian Ah, okay. That was more complicated than you need here, anyway. \$\endgroup\$
    – Davislor
    May 27, 2023 at 20:28
  • \$\begingroup\$ @TheCompile-TimeComedian You might want to hint to your instructor that C++11 is behind the times. The minimum default these days seems to be C++14. \$\endgroup\$
    – pacmaninbw
    May 28, 2023 at 23:55
  • 2
    \$\begingroup\$ @pacmaninbw this \$\endgroup\$ May 29, 2023 at 0:21
  • 1
    \$\begingroup\$ @pacmaninbw According to the project page, modules and a few concepts. Officially, GCC support for C++20 is still “experimental” (as of May 2023). \$\endgroup\$
    – Davislor
    May 29, 2023 at 16:13

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.