C++ Validating Comma Placement in Numerical Input

Question

I'm taking my first cs course and I'm currently learning the different ways to validate numerical inputs. Below is a bool function I wrote to check comma positions, but when I enter 65,000 it thinks the comma is in the wrong place.

bool commas(string input) {
    bool commas = true;
    long len = input.length();
    int counter = 0;

    for(int z = len-1; z >= 0; --z) {
        if(counter == 3) {
            if(input[z] != ',') {
                commas = false;
            counter = 0;
            }
        }
        else {
            if(input[z] == ',') {
                commas = false;
            }
            else {
                ++counter;
            }
        }
    }
    return commas;
}

Wouldn't it be easier if you started from the right side of the string and loop backwards instead of the left? — PaulMcKenzie, Oct 22 '20 at 03:50
The comma in `65,000` is at index `2`. At index `4` it has a `0`. I suggest you consider how far `z` is from the _end_ of the string, not how far it is from the beginning. After all, `1,000,000` has commas at indices `2` and `6`, whereas `10,000,000` has them at indices `3` and `7`. — Nathan Pierson, Oct 22 '20 at 03:51
Also consider the risk of false positives. I assume you don't want something that thinks `1,0,0,0,0,0,0` has commas in the correct spots. How would you detect that? — Nathan Pierson, Oct 22 '20 at 03:54
The reason why you want to start from the right side and go backwards is that each iteration **must** have the correct character. If you start from the left side as you're doing now, you don't know if that `,` after the `5` is valid or not -- you have to wait until you get to some position in the string to say "oh yeah, that comma I saw before really wasn't good". For example `65,00` -- How do you know that the comma is in the correct position if you start from the left side and go forward? Now start from the right side and go backwards -- that comma is immediately known to be bad. — PaulMcKenzie, Oct 22 '20 at 04:03
c++14 also provide a **[digit separator](https://stackoverflow.com/questions/27767781/why-was-the-space-character-not-chosen-for-c14-digit-separators)** feature. — pradeexsu, Oct 22 '20 at 05:34
@PaulMcKenzie I just updated it so that the for loop iterates backward but it's still returning false when commas are in the correct places, any idea why? — userh16xx0, Oct 23 '20 at 04:26
@userh16xx0 -- First, do not change your original post -- add to it with the edited program. Hint, don't use the `z` in the formula. Just simply use another counter that increases on each iteration. Here's why going backwards is easier -- you know *exactly* where the commas are supposed to be -- at iteration 3, 6, 9, 12, etc. if you go backwards. I suggest you work this out on paper. — PaulMcKenzie, Oct 23 '20 at 08:59
I rolled back the edits you made to the original post -- the question now fits what you originally was trying to do. — PaulMcKenzie, Oct 23 '20 at 09:06

PaulMcKenzie · Answer 1 · 2020-10-23T19:50:51.933

The easiest way to figure out if the comma is in the correction position is to go backwards (starting from the rightmost character) within the string.

The reason why this is easier is that if you were to start from the leftmost character and go forward, when you encounter a ,, you don't really know at that point whether that comma is in a valid position. You will only know until later on within the iteration process.

On the other hand, when you go from right-to-left, you know that if you encounter a comma, that comma is in a valid position -- there is no need to wait until you've gone further in the string to determine if the comma is valid.

To do this, it takes an adjustment in the loop to go backwards, and an additional counter to track the current group of 3 digits, since a comma can only occur after a group of 3 digits has been processed.

This is an untested example (except for the simple tests in main):

#include <string>
#include <cctype>
#include <iostream>

bool commas(std::string input)
{
    // if the string is empty, return false
    if (input.empty())
        return false;

    // this keeps count of the digit grouping
    int counter = 0;

    // loop backwards
    for (int z = static_cast<int>(input.size()) - 1; z >= 0; --z)
    {
        // check if the current character is a comma, and if it is in
        // position where commas are supposed to be 
        if (counter == 3)
        {
            if (input[z] != ',')
                return false;

            // reset counter to 0 and process next three digits
            counter = 0;
        }
        else
        // this must be a digit, else error
        if (input[z] == ',')
            return false;
        else
            // go to next digit in group of 3
            ++counter;
    }

    // the first character must be a digit.
    return isdigit(static_cast<unsigned char>(input[0]));
}

int main()
{
    std::string tests[] = { "123,,345", 
                            "123,345",                                 
                            "123,345,678",
                            "12,345",
                            "1223,345",
                            ",123,345",
                            "1",
                            "",
                            "65,00",
                            "123"};

    const int nTests = sizeof(tests) / sizeof(tests[0]);
    for (int i = 0; i < nTests; ++i)
        std::cout << tests[i] << ": " << (commas(tests[i]) ? "good" : "no good") << "\n";
}

Output:

123,,345: no good
123,345: good
123,345,678: good
12,345: good
1223,345: no good
,123,345: no good
1: good
: no good
65,00: no good
123: good

The way this works is simple -- we just increment a counter and see if the current position we're looking at (position z) in the string is a position where a comma must exist.

The count simply counts each group of 3 digits -- when that group of 3 digits has been processed, then the next character (when going backwards) must be a comma, otherwise the string is invalid. We also check if the current position is where a comma cannot be placed.

Note that at the end, we need to check for invalid input like this:

,123

This is simply done by inspecting the first character in the string, and ensuring it is a digit.

This explanation makes sense to me, thank you!! The only problem now is using that logic to fit my code. Since my string is not in an array, it's just one input, I changed a few things. I can't say I did it right because my function isn't returning true for the inputs that have commas in the right places. Are you able to see what I may have missed or interpreted incorrectly? If not that's okay, I appreciate your help! — userh16xx0, Oct 23 '20 at 17:19

C++ Validating Comma Placement in Numerical Input

1 Answers1