9
\$\begingroup\$

I am writing in C++, I have completed a small program which parses CSV files from IUCN and presents the species data using menus. I used a lot of loops. It works, however it is buggy when processing certain input errors.

I would like to know if there are better ways to write this program. Is there a way to use OOP? Would it be helpful to use OOP?

//EndangeredSeals.cpp
#include <iostream>
#include <sstream>
#include <fstream>
#include <string>
#include <vector>
#include <algorithm>

int main()
{
std::string entry;
std::vector<std::vector<std::string> > database;
std::ifstream file("C:\\Users\\MyPC\\Desktop\\Programs\\Seals.csv");

if (file) {
while (std::getline(file, entry)) {
    size_t dbsize = database.size();
    database.resize(dbsize + 1);
    std::istringstream ss(entry);
    std::string field, push_field("");
    bool no_quotes = true;

    while (std::getline(ss, field, ',')) {
        if (static_cast<size_t>(std::count(field.begin(), field.end(), '"')) % 2 != 0) {
            no_quotes = !no_quotes;
            field.erase(remove(field.begin(), field.end(), '\"'), field.end());
        }

        push_field += field + (no_quotes ? "" : ",");

        if (no_quotes) {
            database[dbsize].push_back(push_field);
            push_field.resize(0);
        }
    }
}
}
int dbval =  static_cast<int>(database.size());
int entryval = static_cast<int>(database[0].size());
int sealChoice;
int dataChoice;
int switchChoice;
bool moreData;
bool moreSeal = true;
while (moreSeal == true) {
    moreData = true;
    std::cout << "*********************ENDANGERED SEALS*********************" << std::endl;
    std::cout << "Choose a seal! (1-" << dbval - 1 << ")" << std::endl;
    for (int x = 1; x < database.size(); x++) {
        std::cout << x << ". " << database[x][14] << std::endl;
    }
    std::cout << "\nSeal choice:";
    std::cin >> sealChoice;
    while (sealChoice <= 0 || sealChoice >= dbval || std::cin.fail()){
        std::cout << "\nYou pressed an incorrect key. Try again." << std::endl;
        std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        std::cout << "\nSeal choice:";
        std::cin >> sealChoice;
    }
while (moreData == true) {
    std::cout << "\nWhat data do you want to access about the " << database[sealChoice][14] << "?" << std::endl;
    for (int x = 0; x < entryval; x++) {
        std::cout << x + 1 << "." << database[0][x] << std::endl;
    }
    std::cout << "\nData choice:";
    std::cin >> dataChoice;
     while (dataChoice <= 0 || dataChoice >= entryval || std::cin.fail()){
        std::cout << "You pressed an incorrect key. Try again." << std::endl;
        std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        std::cout << "\nData choice:";
        std::cin >> dataChoice;
    }
    std::cout << database[sealChoice][14] << std::endl;
    std::cout << database[0][dataChoice - 1] << ": " << database[sealChoice][dataChoice - 1] << std::endl;
    std::cout << "\nWould you like to access more data about the " << database[sealChoice][14] << "?" << std::endl;
    std::cout << "1. Yes" << std::endl;
    std::cout << "2. Access data of a different seal" << std::endl;
    std::cout << "3. Quit" << std::endl;
    std::cin >> switchChoice;
    if (switchChoice == 1) {
        moreData = true;
    }
    else if (switchChoice == 2) {
        moreData = false;
        break;
    }
    else if (switchChoice == 3) {
        exit(1);
    }
    else {
        while (switchChoice <= 0 || switchChoice >= 4 || std::cin.fail()){
            std::cout << "You pressed an incorrect key. Try again." << std::endl;
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
            std::cin >> switchChoice;
        }
    }
}
}
return 0;
}
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

Reformat code

Try to use IDE options where you can reformat code, right now it is hard to read. There is everything in one place.

Create small functions

Right now, your code is a one big function. Firstly, divide your code into small functions that are doing one thing. You could end up with 10 or 15 really small functions. After that, thing about functions that are similar to each other, they are part of one bigger responsibility. You might be able to see some "objects" and create appropriate classes.

Hardcoding values

You have code like this

std::ifstream file("C:\\Users\\MyPC\\Desktop\\Programs\\Seals.csv");

This can be done for example using command line arguments. You will be passing path to file when you start running your program, that will be much more flexible.

Next examples of hardcoding values

std::cout << database[sealChoice][14] << std::endl;
while (switchChoice <= 0 || switchChoice >= 4 || std::cin.fail())

Try to give these every number meaningful name, it will much more easier for readers to understand.

Strange code

I cannot get why do you do this

   size_t dbsize = database.size();
   database.resize(dbsize + 1);

Personally, I use resize in only one case. Suppose that I know that my container will be filled with 10 numbers, so before starting pushing into the vector I resize to the mentioned size. You are doing something that will be done better and automatically (no need to write code like this).

Next

int dbval =  static_cast<int>(database.size());
int entryval = static_cast<int>(database[0].size());

I know that works, but I would go with simply

auto dbval =  database.size();
auto entryval = database[0].size();
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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