Input bugging for no reason

Question

I made a program that computes (by a DB itself using fstream) patients of a hospital. It has functions to output (one only for ostream using the iomanip and another to ofstream and fstream) and input (get()). As a requirement, I had to use the Selection Sort and the Binary search that they spend a bad memory (at least the way I had to do the Inary search), and load the data in a rough way. I have implemented the following:

#include <cstring>
#include <iomanip>
class patientType
{
    private:
    char* first_name_t;
    char* second_name_t;
    unsigned id_t;
    public:
    patientType()
    {
    }
    void get()
    {
        char* first_name = new char[128];
        char* second_name = new char[128];
        unsigned id;
        std::cout << "\nType patient's first name: ";
        std::cin >> first_name;
        std::cout << "\nType patient's second name: ";
        std::cin >> second_name;
        std::cout << "\nType patient's ID: ";
        std::cin >> id;
        std::cout << std::endl;
        this->first_name_t = new char[std::strlen(first_name)];
        std::strcpy(this->first_name_t, first_name);
        delete first_name;
        this->second_name_t = new char[std::strlen(second_name)];
        std::strcpy(this->second_name_t, second_name);
        delete second_name;
        this->id_t = id;
    }
    patientType(const char* first_name, const char* second_name, const unsigned id)
    {
        this->first_name_t = new char[std::strlen(first_name)];
        std::strcpy(this->first_name_t, first_name);
        this->second_name_t = new char[std::strlen(second_name)];
        std::strcpy(this->second_name_t, second_name);
        this->id_t = (unsigned)id;
    }
    ~patientType() //De
    {
        this->id_t = 0;
        delete[] this->first_name_t;
        delete[] this->second_name_t;
    }
    friend std::ostream& operator<<(std::ostream& os, patientType& patient)
    {
        os << patient.first_name() << std::left << std::setw(3) << ' ' << patient.second_name() << std::left << std::setw(3) << ' ' << patient.id() << std::endl;
        return os;
    }
    void save(std::ofstream& ofs)
    {
         ofs << this->first_name() << "\t" << this->second_name() << "\t" << this->id() << "\n";
    }
    void save(std::fstream& ofs)
    {
        ofs << this->first_name() << "\t" << this->second_name() << "\t" << this->id() << "\n";
    }
    void operator=(const patientType& patient)
    {
        this->first_name_t = new char[std::strlen(patient.first_name_t)];
        std::strcpy(this->first_name_t, patient.first_name_t);
        this->second_name_t = new char[std::strlen(patient.second_name_t)];
        std::strcpy(this->second_name_t, patient.second_name_t);
        this->id_t = (unsigned)patient.id_t;
    }
    const char* first_name() {return const_cast<const char*>(this->first_name_t);}
    const char* second_name() {return const_cast<const char*>(this->second_name_t);}
    const unsigned id() {return (const unsigned)id_t;}
};
#include <vector>
template<class T> std::vector<unsigned> binary_search(T* var, T term, unsigned size)
{
    std::vector<unsigned> ret;
    unsigned mid = size / 2;
    for(unsigned i = 0; i < mid; i++) {if(var[i] == term) ret.push_back(i);}
    for(unsigned i = mid; i < size; i++) {if(var[i] == term) ret.push_back(i);}
    return ret;
}

template<class T, class BinaryCompare> std::vector<unsigned> binary_search(T* var, T term, unsigned size, BinaryCompare compare_operation)
{
    std::vector<unsigned> ret;
    unsigned mid = size / 2;
    for(unsigned i = 0; i < mid; i++) {if(compare_operation(var[i],term)) ret.push_back(i);}
    for(unsigned i = mid; i < size; i++) {if(compare_operation(var[i],term)) ret.push_back(i);}
    return ret;
}
template <typename T> void swap(T& var, T& var1)
{
    T temp = var;
    var = var1;
    var1 = temp;
}
template <typename T> void selection_sort(T* var, unsigned size, unsigned pos = 0) //1 3 5 2 4; tamanho 5
{
    if(!(pos >= size))
    {
        for(int i = pos+1; i < size; i++)
        {
            if(var[pos] > var[i]) swap(var[pos], var[i]);
        }
        selection_sort(var, size, pos+1);
    }
    else return;
}
template <typename T, typename BinaryCompare>void selection_sort(T* var, unsigned size, BinaryCompare compare_operation, unsigned pos = 0)
{
    if(!(pos >= size))
    {
        for(int i = pos+1; i < size; i++)
        {
            if(compare_operation(var[pos], var[i])) swap(var[pos], var[i]);
        }
        selection_sort(var, size, compare_operation, pos+1);
    }
    else return;
}

So I did the main():

#include <iostream>
#include <fstream>
#include "patientType.hpp"
#include "search.hpp"
#include "sort.hpp"
#include <string>
#include <sstream>

bool file_exists(const char* file)
{
    std::ifstream file_(file);
    if(!file_.good() || file_.fail() || file_.bad()) return false; else return true;
}
bool empty_file(const char* file)
{
    std::ifstream file_(file);
    return file_.peek() == std::ifstream::traits_type::eof();
}

int main()
{
    while(true)
    {
        unsigned option = 0;
        if(!file_exists("patient.db") || empty_file("patient.db"))
        {
            std::ofstream file("patient.db");
            std::cout << "1. Add patient\n";
            std::cout << "2. Exit\n> ";
            std::cin >> option;
            switch(option)
            {
                case 1:{patientType patient; patient.get(); patient.save(file); break;}
                case 2: {exit(0);break;}
                default: {std::cout << "Input a valid option.\n";break;}
            }
        }
        else
        {
            std::string file_content;
            std::fstream file("patient.db");
            unsigned file_size = 0;
            while(std::getline(file, file_content)) {if(!file_content.empty()) file_size++;}
            file.close();
            file.open("patient.db");
            patientType* patients = new patientType[file_size];
            int instance = 0;
            while(std::getline(file, file_content))
            {
                std::string params[3];
                std::stringstream A(file_content);
                for(int i = 0; std::getline(A, file_content, '\t');) {if(!file_content.empty()){params[i] = file_content;i++;}}
                patientType A_(params[0].c_str(), params[1].c_str(), std::stoi(params[2]));
                patients[instance] = A_;
                instance++;
            }
            std::cout << "1. Print patient\n2. Add a patient\n3. Sort patients by last name\n4. Sort patients by ID\n5. Search patient by last name\n6. Search patient by ID\n7. Exit the program\n> ";
            std::cin >> option;
            switch(option)
            {
                case 1:
                {
                    unsigned index;
                    std::cout << "\nSelect index (in range 1 to " << file_size << ").\n> ";
                    std::cin>> index;
                    if(index < 1 || index > file_size)
                    {
                        exit(-1);
                    }
                    else std::cout <<"\n" << std::left << "1st name" << std::left << std::setw(3) << " 2nd name" << std::left << std::setw(3) << " ID\n" << patients[index - 1] << "\n";
                    break;
                }
                case 2:
                {
                    file.close();
                    patientType patient;
                    patient.get();
                    std::ofstream file_("patient.db", std::ios::app);
                    patient.save(file_);
                    break;
                }
                case 3:
                {
                    selection_sort(patients, file_size, [](patientType& first, patientType& second){return first.second_name() > second.second_name();});
                    break;
                }
                case 4:
                {
                    selection_sort(patients, file_size, [](patientType& first, patientType& second){return first.id() > second.id();});
                    break;
                }
                case 5:
                {
                    unsigned term;
                    std::cout << "Input the ID term: ";
                    std::cin >> term;
                    std::vector<unsigned> occurrences = binary_search(patients, patientType("","",term), file_size, [](patientType& first, patientType& second){return first.id() == second.id();});
                    std::cout << "Found " << occurrences.size() << " occurrences.\n";
                    for(auto a : occurrences)
                    {
                        std::cout << patients[a];
                        std::cout << "\nAt " << a << "\n";
                    }
                    std::cout << std::endl;
                    break;
                }
                case 6:
                {
                    char* term = new char[128];
                    std::cout << "Input the last name term: ";
                    std::cin >> term;
                    std::vector<unsigned> occurrences = binary_search(patients, patientType("", term,0), file_size, [](patientType& first, patientType& second){return first.second_name() == second.second_name();});
                    std::cout << "Found " << occurrences.size() << " occurrences.\n";
                    for(auto a : occurrences)
                    {
                        std::cout << patients[a];
                        std::cout << "\nAt " << a << "\n";
                    }
                    std::cout << std::endl;
                    delete[] term;
                    break;
                }
                case 7:
                {
                    exit(0);
                }
            }
        }
    }
}

If you even test it, it gives way in various functions, in debug gives SIGTRAP several times and out of the blue, when asking for an input several times it crashes - to. The program (with the requirements) would be slow, but probably not bugle. Where is the error? How to fix?

Answer 1

The problem is in the excerpts you allocate a new string thus:

new char[std::strlen(first_name)];

Should be:

new char[std::strlen(first_name) + 1];

Without the + 1 you are not considering the null character of the string \0, which causes no problem in your program until the destructor of patientType be called. And it runs when you add new patients and when you list them.

---

Other details:

1. In function patientType::get() the variables first_name and second_name should be deleted using delete [] (like you did in the destructor) and not just delete.
   
2. In patientType::operator=(const patientType& patient) you must check and delete the previous values (or clear) of the strings before allocating new strings.
3. When you do return first.second_name() == second.second_name(); you are comparing pointers, to compare the value of strings use strcmp().
4. And lastly, when you call the function binary_search(T* var, T term, unsigned size, BinaryCompare compare_operation) you are copying the object T term. This means that every time you call this function, the compiler will call the construtor de cópia (that you have not set), causing the strings to not be properly allocated. A simple solution is to pass the term by reference (T& term).

---

Another simple solution to these problems is to not allocate dynamically when there is no need. For example, in the class you do:

// Na classe:
char* first_name_t; 
char* second_name_t;

// Em get
char* first_name = new char[128];
char* second_name = new char[128];

You can just do:

// Na classe:
char first_name_t[128]; 
char second_name_t[128];

// Em get
char first_name[128];
char second_name[128];

And limit that the user does not enter names larger than 127 characters. With this you no longer need to worry about dislocating. The disadvantage of doing so is that each name will always occupy 128 char, rather than just the necessary.

---

I imagine you can’t use std::string, but this is an example of why you should use it (it would save you all that trouble). But anyway, when you’re managing the memory manually, I’d recommend checking to see if your class meets Rule of Three (or Five in C++11).