Input bugging for no reason

Question

Input bugging for no reason

I made a program that computes (by a DB of its own using fstream) patients from a hospital. It has functions for output (one for ostream using iomanip and one for ofstream and fstream) and input (get()). As a requirement, I had to use selection sort and binary search which spend a hell of a lot of memory (at least the way I had to do binary search), as well as loading the data in a raw way. I 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 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 test yourself, it gives stick in several functions, in debug it gives SIGTRAP several times and out of nowhere, when asking for an input several times it crashes - to. The program (with the requirements) would slow down, but probably wouldn't bug. Where's the mistake? How to arrange?

2

c++algoritmo classificação busca

Author: Victor Stafusa, 2014-05-03

Source

1 answers

score 1 · Accepted Answer

The problem is in the snippets that you allocate a new string like this:

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 problems in your program until the destructor of patientType is called. And this runs when you add new patients and when you list them.

Other details:

in the function patientType::get() the variables first_name and second_name must be deleted using delete [] (equal you did in the destroyer) and not just delete.
in patientType::operator=(const patientType& patient) you must check and delete the previous values (or clear) of the strings before allocating new strings.
when you do return first.second_name() == second.second_name(); you are comparing pointers, to compare the value of strings use strcmp().
and finally, 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 (which you don't have the strings are not properly allocated. A simple solution is to pass the term by reference (T& term).

Another simple solution to these problems is not to dynamically allocate 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 simply 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 report names larger than 127 characters. With this you no longer have to worry about relocating. To disadvantage of doing this way is that each name will always occupy 128 char, rather than occupying only 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 work). But anyway, when you are managing memory manually, I would recommend checking if your class meets the Rule of three (or five in C++11).