Insertion sort with high OOP and error processing

Question

The input file contains data of one of two types: integers or strings. Data recorded in the column (each line of the file is a new element). Strings can contain any non-whitespace characters. The result of the program should be a new file with the contents of the input file. Sorted in ascending or descending order. To sort the rows, use lexicographical order (by character codes). All possible types of errors must be processed. The program should not "fall". If after an error it is not possible to continue the execution, the program must report this user with a reason for failure.

How do you like my code? The main task was to implement OOP.

file_sort.cpp

#include "file_sorter.h"

/*
Usage:
sort-it.exe in.txt out.txt -i -a (Ascending insertion sort for integers)
sort-it.exe in.txt out.txt -i -d (Descending insertion sort for integers)
sort-it.exe in.txt out.txt -s -a (Ascending insertion sort for strings)
sort-it.exe in.txt out.txt -s -d (Descending insertion sort for strings)
*/

bool file_sort::a_bigger_b_int(int a, int b)
{
    return (a > b);
}
bool file_sort::a_less_b_int(int a, int b)
{
    return (a < b);
}
bool file_sort::a_less_b_string(std::string a, std::string b)
{
    return a.compare(b) < 0;
}
bool file_sort::a_bigger_b_string(std::string a, std::string b)
{
    return a.compare(b) > 0;
}

file_sort::file_sort(std::string read_file, std::string output_file, std::string type_content, std::string type_of_sorting)
{
    this->read_file_name.append(read_file);
    this->write_file_name.append(output_file);
    this->type_of_content_string.append(type_content);
    this->type_of_sorting_string.append(type_of_sorting);
}
file_sort::file_sort(int args_number, char *args[])
{
    if (args_number != 5) {
        std::cout << "Wrong arguments quantity!\n";
        exit(0);
    }
    else {
        *this = file_sort(args[1], args[2], args[3], args[4]);
    }
}
file_sort::file_sort()
{
    std::cout << "No any arguments!\n";
    exit(0);
}
file_sort::~file_sort()
{
}
void file_sort::sort_the_file()
{
    if (!read_file()) {
        std::cout << "File doesn't exist!\n";
    }
    else {
        read_file();
        sort_the_content();
        write_in_file();
    }
}
void file_sort::sort_the_content()
{   
    if (!are_normal_arguments(type_of_content_string, type_of_sorting_string)) {
        std::cout << "Wrong arguments!\n";
        exit(0);
    }
    current_size = get_current_size();
    if (is_type_of_content_number()) {
        make_to_number();
        if (type_of_sorting_string.compare("-a") == 0)
            sort_number(&file_sort::a_less_b_int);
        else
            sort_number(&file_sort::a_bigger_b_int);
    }
    else
        if (type_of_sorting_string.compare("-a") == 0)
            sort_string(&file_sort::a_less_b_string);
        else
            sort_string(&file_sort::a_bigger_b_string);
}
void file_sort::sort_string(bool (file_sort::*comprassion_function)(std::string, std::string))
{
    for (int i = 1; i < current_size; i++) {
        std::string current_element = string_text[i];
        int j;
        for (j = i; j > 0 && (this->*comprassion_function)(current_element, string_text[j - 1]); j--)
            string_text[j] = string_text[j - 1];
        if ((this->*comprassion_function)(current_element, string_text[j]))
            string_text[j] = current_element;
    }
}
void file_sort::sort_number(bool (file_sort::*comprassion_function)(int, int))
{
    for (int i = 1; i < current_size; i++) {
        int current_element = integer_text[i];
        int j;
        for (j = i; j > 0 && (this->*comprassion_function)(current_element, integer_text[j - 1]); j--)
            integer_text[j] = integer_text[j - 1];
        if ((this->*comprassion_function)(current_element, integer_text[j]))
            integer_text[j] = current_element;
    }
}
bool file_sort::read_file()
{
    std::ifstream file_in(read_file_name);
    if (!file_in.is_open()) {
        return false;
    }
    else {
        for (int i = 0; i < max_size, !file_in.eof(); i++) {
            file_in >> string_text[i];
        }
        return true;
    }
}
void file_sort::write_in_file()
{
    std::ofstream file_out(write_file_name);
    for (int i = 0; i < current_size; i++)
        if (is_type_of_content_number())
            file_out << integer_text[i] << std::endl;
        else
            file_out << string_text[i] << std::endl;
}
void file_sort::make_to_number()
{
    if (!is_all_content_number()) {
        std::cout << "Content contains not number!";
        exit(0);
    }
    for (int i = 0; i < current_size; i++) {
        integer_text[i] = atoi(string_text[i].c_str());
    }
}
int file_sort::get_current_size()
{   
    for (int i = 0; i < max_size; i++) {
        if (string_text[i].empty())
            return i;
    }
    return max_size;
}
bool file_sort::is_type_of_content_number()
{
    return type_of_content_string != "-s";
}
bool file_sort::is_all_content_number()
{
    for (int i = 0; i < current_size; i++) {
        for (int j = 0; (size_t)j < string_text[i].length(); j++) {
            if (!isdigit(string_text[i][j]))
                return false;
        }
    }
    return true;
}
bool file_sort::are_normal_arguments(std::string type_content, std::string type_of_sorting) {
    if (type_content != "-s" && type_content != "-i")
        return false;
    if (type_of_sorting != "-a" && type_of_sorting != "-d")
        return false;
    return true;
}

Answer 1

Let's start by considering how this might look if you were just using std::sort instead of writing a sort algorithm of your own.

Such code might look something like this:

#include <iostream>
#include <string>
#include <iterator>
#include <vector>
#include <string>
#include <fstream>
#include <algorithm>
#include <functional>

enum class direction { ascending, descending };
enum class type { string, integer };

void sort_s(std::istream &input, std::ostream &output, direction d) {
    std::vector<std::string> strings{
        std::istream_iterator<std::string>(input),
        std::istream_iterator<std::string>()
    };

    if (d == direction::descending)
        std::sort(strings.begin(), strings.end(), std::greater<std::string>());
    else
        std::sort(strings.begin(), strings.end());
    for (auto const &s : strings)
        output << s << '\n';
}

void sort_i(std::istream &input, std::ostream &output, direction d) {
    std::vector<int> ints{
        std::istream_iterator<int>(input),
        std::istream_iterator<int>()
    };

    if (d == direction::descending)
        std::sort(ints.begin(), ints.end(), std::greater<int>());
    else
        std::sort(ints.begin(), ints.end());
    for (auto const &i : ints)
        output << i << '\n';
}

int main(int argc, char **argv) {
    direction d = direction::ascending;
    type t = type::string;

    std::vector<std::string> args{ argv + 1, argv + argc };
    std::vector<std::string> filenames;

    for (auto && arg : args) {
        if (arg[0] == '-') {
            switch (arg[1]) {
            case 'd': d = direction::descending;    break;
            case 'a': d = direction::ascending;     break;
            case 'i': t = type::integer;            break;
            case 's': t = type::string;             break;
            default:
                std::cerr << "unknown flag: " << arg[1] << '\n';
                return EXIT_FAILURE;
            }
        }
        else filenames.push_back(arg);
    }

    if (filenames.size() != 2) {
        std::cerr << "Usage: sort <infile> <outfile> [-i] [-s] [-d] [-a]\n";
        return EXIT_FAILURE;
    }

    std::ifstream input(filenames[0]);
    std::ofstream output(filenames[1]);

    if (t == type::string)
        sort_s(input, output, d);
    else
        sort_i(input, output, d);
}

At least as I see things, code using an insertion sort should look and work fairly similarly, except that inside of sort_i or sort_s, it would call an insertion_sort instead of std::sort. Everything else can stay the same--and that's as things should be--changing the sort algorithm should only affect the implementation of that algorithm, not anything else at all.

Now, that's certainly not to say that everything about this code is perfect. In fact, a number of things are undoubtedly open to improvement (of course). Nonetheless, there's no reason the sort algorithm should be mashed together with the command line argument processing, the file processing, etc.

Review Proper

I almost hate to get into the details of the code itself. Quite a bit of it goes in what I'd consider exactly the wrong direction. I'll give a few examples though.

Overall Structure

You seem to have mashed essentially everything together into a file_sort class. This looks suspiciously like a god class, a well-known anti-pattern.

Function names

bool file_sort::a_bigger_b_int(int a, int b)

I'd prefer not to put the type into the name. We can easily see that this takes int parameters (and that the string version takes string parameters).

Many of the names are also needlessly chatty. is_type_of_content_number would be better (IMO) as something like is_numeric.

Useless functions

You've included a dtor for your file_sort class, but it merely has an empty body. There used to be a few cases where this couldn't be avoided, but most of those are gone since C++11. It doesn't look like it's accomplishing anything here.

Use standard algorithms where applicable

Using std::sort instead of writing your own sort routine is one example. Another would be writing an explicit string to verify that a string contains only digits:

bool file_sort::is_all_content_number()
{
    for (int i = 0; i < current_size; i++) {
        for (int j = 0; (size_t)j < string_text[i].length(); j++) {
            if (!isdigit(string_text[i][j]))
                return false;
        }
    }
    return true;
}

This would be better off using std::all_of.

Use of (effectively) globals

Many of your member functions (such as is_all_content_number) use class variables. When possible, it's generally better to create pure functions that take some set of input parameters, and produce a result that depends only on those input parameters.

Due to the excessive size of your class, this currently uses what are effectively global variables--the file_sort class is virtually the entire program, so class variables are effectively global.

Answer 2

I'll shoot for your main incentive to write this code - the lower level C++ details have been attacked by Jerry already.

The main task was to implement OOP.

I see not a single hint of OOP (object oriented programming) in this code snippet.

Some pointers (sic):

• Red flag: You are often branching based on the type of some object - this is patently anti-OOP, and a prime spot to start becoming "OOP". To go from if TYPE==A then do_something_to_A(obj) else do_something_to_B(obj) to OOP, you would create two classes with a common base class Z, methods A::something and B::something, and then only call obj.something. The caller would, ideally, never look at the type of the actual item, but handle them as Z.
• The most important bit of the acronym OOP is the middle O, "oriented". That means that you are not only using object syntax somewhere in your code, but actually having a perceptible object/class design. For sorting, you could have Items that can be sorted, breaking down into IntItem and CharItem, some kind of SortAlgorithm which has a generic sort method; some concrete implementations (BubbleSortAlgorithm, QuickSortAlgorithm etc.). Then you want to make sure to respect some general principles like maximizing cohesion, minimizing coupling and such. For example, each XXXAlgorithm would only ever handle Item, not one of the concrete implementations, and so on and so forth. Obviously, all the while trying not to overdesign or to overcomplicate things (at which point experience really pays off - it will come with time).

So...

How do you like my code?

If you fixed the issues called out by Jerry, I'd probably accept it as OK'ish C++ code.

If you wanted to sell it to me as "OOP" code, blatantly "no".

Don't take that as a too harsh of a judgement. Go back, start over with a class design for sorting (at which point you don't need to think "C++" yet), skip anything that looks overly procedural (if, globals, etc.), get good cohesion and little coupling, use design patterns where appropriate (for example, Strategy for your sort algorithms, if you indeed want to model multiple algorithms) and so on.

Answer 3

When an error is encountered, it is customary to print the error message to std::cerr so as not to contaminate std::cout. Also, exit with a non-zero status code to indicate an error. Exiting with 0 indicates success.