HiveBrain v1.2.0
Get Started
← Back to all entries
debugcppModerate

Are these try/catch blocks cluttering the code?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
blocksthesethearecatchclutteringcodetry

Problem

The following program reads a stream and creates a two dimensional std::vector.

I'm trying to find an elegant way to handle the errors and recover from them as much as possible while still reporting the problems encountered.

It feels as though my nice clean functions are getting more and more cluttered as I add the error handling.

Is this the best way?

#include 
#include 
#include 
#include 
#include 
#include 

namespace {

    using Grid_row = std::vector;
    using Grid = std::vector;

    class Partial_value : public std::runtime_error {
    public:
        Partial_value(std::string text, int val) : std::runtime_error{text}, val{val} {}
        int val{};
    };

    int convert(const std::string& text)
    {
        int val{};
        try {
            val = std::stoi(text);
        }
        catch(const std::exception& e) {
            throw Partial_value{"Due to error: \"" + std::string{e.what()} +"\"; Could not convert " + text + " to integer. Value was converted to 0.", val};
        }
        std::ostringstream oss{};
        oss > (std::istream& is, Grid_row& grid_row)
    {
        std::string term{};
        while(is >> term) {
            int val{};
            try {
                val = convert(term);
            }
            catch(const Partial_value& e) {
                val = e.val;
                std::cerr > (std::istream& is, Grid& grid)
    {
        std::string line{};
        while(std::getline(is, line)) {
            std::istringstream is_line{line};
            Grid_row grid_row{};
            is_line >> grid_row;
            grid.push_back(grid_row);
        }
        return is;
    }

    std::ostream& operator > grid;
    std::cout << grid;
    return 0;
}


Produces:

```
Could not fully convert 7.5 to integer. Value was converted to 7.
Due to error: "invalid stoi argument"; Could not convert -Z to integer. Value was converted to 0.
3 -1 -3 -1 -4
40 7 3 1 1
10 -2 0 1 1
10

Solution

Your convert function should be pure.

convert should be doing one thing and one thing only, and that is to convert the given string to int. All those checks you do are not useful because they can easily be performed in the function that calls convert.

int convert(const std::string& text) {
    return std::stoi(text);
}


Your convert function is not needed.

Seeing as after changing it to a pure function, it is now acting as a wrapper to another function call, you don't need it anymore, and it can now be replaced by the function it calls, std::stoi.

Move convert logic elsewhere

Move all the logic from within the convert method to your overloaded function, std::istream& operator >> (std::istream& is, Grid_row& grid_row).

std::istream& operator >> (std::istream& is, Grid_row& grid_row) {
    std::size_t fin;
    int val;

    for (std::string term; is >> term; ) {
        try {
            val = std::stoi(term, &fin);
            if (fin != term.size()) {
                std::cerr << "Could not fully convert " + text + " to integer. Value was converted to " + text.substr(0, fin) + "." << '\n';
            }
        } catch(const std::exception& e) {
            std::cerr << "Due to error: \"" + std::string{e.what()} +"\"; Could not convert " + text + " to integer. Value was converted to 0." << '\n';
        }

        grid_row.push_back(val);

    }
    return is;
}


Now all you have is one try-catch, which is really all that is needed

Code Snippets

int convert(const std::string& text) {
    return std::stoi(text);
}
std::istream& operator >> (std::istream& is, Grid_row& grid_row) {
    std::size_t fin;
    int val;

    for (std::string term; is >> term; ) {
        try {
            val = std::stoi(term, &fin);
            if (fin != term.size()) {
                std::cerr << "Could not fully convert " + text + " to integer. Value was converted to " + text.substr(0, fin) + "." << '\n';
            }
        } catch(const std::exception& e) {
            std::cerr << "Due to error: \"" + std::string{e.what()} +"\"; Could not convert " + text + " to integer. Value was converted to 0." << '\n';
        }

        grid_row.push_back(val);

    }
    return is;
}

Context

StackExchange Code Review Q#149645, answer score: 11

Revisions (0)

No revisions yet.