Easy integer input

Question

I'm writing this app that's based in the terminal, and of course there's a lot of input involved, especially numerical input. I've decided to use the method of reading an entire line of input and converting into a number and displaying an error message if the input is out of bounds or just straight-up invalid. This happens inside and infinite loop so that the user can keep trying until they get it right.

Having to do this so many times in my code has resulted in me having to move it to function in a namespace like so:

io_helpers.h

#ifndef IO_HELPERS_H
#define IO_HELPERS_H

#include <string>

namespace easyio
{
    bool fileExists(const std::string& filename);
    int readInt(const std::string& prompt, int min, int max, const std::string& errorMsg);
}

#endif

io_helpers.cpp

#include "io_helpers.h"
#include <fstream>
#include <iostream>

using namespace std;

bool easyio::fileExists(const string& filename)
{
    ifstream fin(filename);
    if (fin.is_open())
    {
        return true;
    }
    else
    {
        return false;
    }
}

int easyio::readInt(const string& prompt, int min, int max, const string& errorMsg = "invalid input!")
{
    while (true)
    {
        try
        {
            string input;
            cout << prompt << ": ";
            getline(cin >> ws, input);
            int number = stoi(input);
            if (number < min || number > max)
                throw exception();
            return number;
        }
        catch (exception& e)
        {
            cout << errorMsg << endl;
        }
    }
}

Is this method of input the best method though? Is there anything I can do to improve the user experience or the performance? And on a side-note, should this be a static method in a class or a function in a namespace, or does it not matter?

Answer 1

Don't use an if statement when both branches do the same thing.

if (fin.is_open())
{
    return true;
}
else
{
    return false;
}

Easier to just return.

return fin.is_open();

Using exceptions for control flow is not a good idea:

        if (number < min || number > max)
            throw exception();

Exceptions are good for moving control from the current location to a location you can not know about at the time of development. This is usually because you can not fix the problem locally.

In this situation you can fix the problem locally and thus error codes are better (as long as they do not cross any public API).

    try {
        // Get input:

        if (number >= min && number <= max) {
            return number;
        }
    }
    catch (std::exception const&) { // catch by const reference
        // Ignore exceptions.
    }
    std::cout << errorMsg << "\n";

Personally I don't see the point in dropping white space here: cin >> ws as all other routines are going to drop white space including stoi(). It just makes the code harder to read.

You also don't check for garbage on the end of the line. What happens if I type 5x5<enter> does that have meaning? Persoanlly I would do some more checking:

std::string  line;
std::getline(std::cin, line);

std::stringstream linestream(line);
int value;
char x;

if ((linestream >> value) && (!linestream >> x))
{
     // We only enter here iff
     //    1) We successfully read an integer from the stream.
     //    2) We failed to read a following (non white space) character.
     // This means there was only a number on the line.
     return value;
}

Usual warnings about this:

using namespace std;

Best practive is to always use {} on sub statements:

        if (number < min || number > max)
            throw exception();

I would have written:

        if (number < min || number > max) {
            throw exception();
        }