0
\$\begingroup\$

Based on previous question , I have implemented all suggestions. also, i have fixed all unnecessary copies for std::string objects in the code.

Is there any thing else can be done to this code?

#include <iostream>
#include <string>
#include <regex>
#include <cctype>
#include <limits>
#include <sstream>
#include <cassert>
#include <type_traits>

// C::B : error: 'stoi' is not a member of 'std'!
int string_to_int(std::string::const_iterator first, 
                  const std::string::const_iterator last)
{
    int value = 0;
    if(first == last) return value;

    const auto sign = *first;
    if (*first == '-' || *first == '+')
        first++;

    while (first != last)
    {
        value = value * 10 - (*first++ - '0');
    }

    if (sign != '-')
        value = -value;

    return value;
}

int is_RC_type(const std::string& in)
{
    const auto index = in.find_first_of("C");
    if (index == std::string::npos) return false;

    const auto& row = in.substr(1, index - 1);
    const auto& col = in.substr(index + 1);

    static const int Min = 1;
    static const int Max = 999999;
    static const std::regex RC{"^R[0-9]+C[0-9]+$"};

    const auto colVal = string_to_int(col.cbegin(), col.cend());
    const auto rowVal = string_to_int(row.cbegin(), row.cend());

    return  col[0] != '0'
         && row[0] != '0'
         && rowVal >= Min
         && rowVal <= Max
         && colVal >= Min
         && colVal <= Max
         && std::regex_match(in, RC);
}

bool is_Cell_type(const std::string& in)
{
    const auto index = in.find_first_of("0123456789");
    if (index == std::string::npos) return false;

    const auto& row = in.substr(0, index);
    const auto& col = in.substr(index);

    static const int CMin = 1;
    static const int CMax = 999999;

    static const std::string AlphaMin{"A"};
    static const std::string AlphaMax{"BDWGM"};
    static const std::regex Cell{"^[A-Z]+[0-9]+$"};

    static const auto RMin = string_to_int(AlphaMin.cbegin(), AlphaMin.cend());
    static const auto RMax = string_to_int(AlphaMax.cbegin(), AlphaMax.cend());

    const auto colVal = string_to_int(col.cbegin(), col.cend());
    const auto rowVal = string_to_int(row.cbegin(), row.cend());

    return col[0] != '0'
        && colVal >= CMin
        && colVal <= CMax
        && rowVal >= RMin
        && rowVal <= RMax
        && std::regex_match(in, Cell);
}

template <typename Input, typename Prompt, typename Handle>
Input read_input_impt(Prompt&& prompt, Handle&& handle_input, 
                      std::istream& is = std::cin, std::ostream& os = std::cout)
{
    Input input{};
    bool is_not_valid = false;
    static const std::string Error{"Not valid input.\n"};

    do
    {
        os << std::forward<Prompt>(prompt) << std::flush;

        is_not_valid = std::forward<Handle>(handle_input)(input, is);

        if (is_not_valid)
        {
            os << Error;
        }

        is.clear();
        is.ignore(std::cin.rdbuf()->in_avail());
    } while (is_not_valid);

    return input;
}

template <typename Input, typename Prompt>
std::enable_if_t<std::is_integral<Input>::value && 
!std::numeric_limits<Input>::is_signed, Input>
read_input(Prompt&& prompt)
{
    static auto int_input = [] (auto& in, std::istream& is)
    {
        static const std::regex Numbers("^[1-9]+$");
        std::string s;
        std::getline(is, s);
        std::stringstream stream{s};
        return is.fail()
            || !std::regex_match(stream.str(), Numbers)
            || !(stream >> in)
            || (1 < in && in > 100000);
    };

   return read_input_impt<Input>(std::forward<Prompt>(prompt), int_input);
}

template <typename Input, typename Prompt>
std::enable_if_t<std::is_same<std::decay_t<Input>, std::string>::value, Input>
read_input(Prompt&& prompt)
{
    static auto string_input = [] (auto& in, std::istream& is)
    {
        std::getline(is, in);
        return  is.fail() || !(is_Cell_type(in) != is_RC_type(in));
    };

    return read_input_impt<Input>(std::forward<Prompt>(prompt), string_input);
}

int main()
{
    auto n = read_input<unsigned int>("Enter a number of entries: ");

    std::cout << "\n\n\tWelcome\n\n";

    do
    {
        const auto& word = read_input<std::string>("Enter your choice: ");

        if (is_RC_type(word))
        {
            const auto index = word.find_first_of("C");
            assert(index != std::string::npos);
            const auto& row = word.substr(1, index - 1);
            const auto& col = word.substr(index + 1);

            auto c = string_to_int(col.cbegin(), col.cend());
            std::string result;

            do
                result =  static_cast<char>('A' + c % 26 - 1) + result;
            while (c /= 26);

            std::cout << "\n\nResult: " << result << row << "\n\n";
        }
        else
        {
            const auto index = word.find_first_of("0123456789");
            assert(index != std::string::npos);
            const auto& row = word.substr(index);
            const auto& col = word.substr(0, index);
            int c = 0;
            for (const auto& ch : col)
            {
                c = 26 * c + (ch - 'A' + 1);
            }

            std::cout << "\n\nResult: R" << row << "C" << c << "\n\n";
        }
    } while (--n);
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

You're not making the most out of your regex's. Regex's are good at checking for pattern matches, but they also have support for capturing elements of the expression. If you use the regex's fully, I believe it will simplify your code significantly.

Consider this expression:

regex pattern("(?:R([0-9]+)C([0-9]+))$|(?:([A-Z]+)([0-9]+))$");

It checks for either R<num>C<num> or <letters><numbers>. The brackets define capture groups for each of those elements (?:) groups elements but doesn't capture the group.

Using the above regex, along with smatch, I wrote a little method to demonstrate an alternate approach to parsing the input:

string strings[]{ "R23C55", "AB235", "R50C200", "CAA425" };
regex pattern("(?:R([0-9]+)C([0-9]+))$|(?:([A-Z]+)([0-9]+))$");
smatch matches;

for (const auto &s : strings) {
    if (regex_match(s, matches, pattern)) {

        if (matches[1].matched) {
            // Capture group 1 matches, so this is R23C55 format
            string numericRow = matches[1].str();
            string numericColumn = matches[2].str();
        }
        else {
            // Capture group 1 didn't match, this must be AB235 format
            string alphaColumn = matches[3].str();
            string alphaNumRow = matches[4].str();
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.