Dynamically allocated array always set to size 1

Question

I am making a program to calculate the value of words for a friend who is interested in numerology.

In my program, I ask the user how many words they want to calculate, so I can dynamically allocate the size of the array to store the words.

It works perfectly the first time they run the program, but when I ask if they want to test more words, and if they hit yes, it reruns the while loop and it asks them for how many words to evaluate, but this time, no matter what number they type, it always sets the size of the array to 1, due to the number variable being set to 1 and not changing after user input. My friend has to rerun the program to make it work again.

I really want to fix this problem to make it easier for my friend.

(Yes, I do plan on fixing my code, and if you have seen this code in another question, that was me, and yes, I am going to improve my code based on the suggestions I got, but I want it to at least work before I improve on it.)

Here is my code:

#include <iostream>
#include <string>
#include <sstream>

void clear()
{
    system("clear");
}

int main(int argc, char* argv[])
{
    int number{1};
    int total{0};
    int value{0};
    std::string number_str;
    std::string response;
    std::stringstream ss;
    bool isDigit;


    while (true)
    {
        clear();

        number = 1;
        total = 0;
        value = 0;
        number_str = "";
        response = "";
        isDigit = true;

        clear();
        std::cout << "How many words to evalute? (Default: 1):\n> ";
        //std::cin >> number;
        std::getline(std::cin, number_str);

        //std::cin.ignore();
        clear();

        for (int i = 0; i < number_str.size(); ++i)
        {
            if (!(std::isdigit(number_str[i])))
                isDigit = false;
                break;
        }

        if (isDigit)
        {
            if (number_str.empty()) {
                number = 1;
            } else {
                ss << number_str;
                ss >> number;
            }
        }


        if (std::isdigit(number)) {
            if (number <= 0) {
                number = 1;
            }
        }

        std::string* pPhrase = new std::string[number];
        int* pValue = new int[number]{}; // Initialized each element to 0

        for (int i = 0; i < number; ++i) // could replace "number" with "sizeof(pPhrase)/sizeof(pPhrase[0])"
        {
            (i == 0) ? std::cout << "Enter Word #" << i+1 << " (or type your full phrase):\n> " :
                                            std::cout << "Enter Word #" << i+1 << ":\n> ";
            std::cin >> pPhrase[i];

            for (char j : pPhrase[i])
            {
                value = 0;
                if (std::isalnum(j) && j != '0')
                {
                    if (std::isalpha(j))
                        j = std::tolower(j);

                    
                } else {
                    continue;
                }

                if (j == 'a' || j == 'i' || j == 'j'
                        || j == 'q' || j == 'y' || j == '1')
                    value += 1;

                if (j == 'b' || j == 'k' || j == 'r' || j == '2')
                    value += 2;

                if (j == 'c' || j == 'g' || j == 'l'
                        || j == 's' || j == '3')
                    value += 3;

                if (j == 'd' || j == 'm' || j == 't' || j == '4')
                    value += 4;

                if (j == 'e' || j == 'h' || j == 'n'
                        || j == 'x' || j == '5')
                    value += 5;

                if (j == 'u' || j == 'v' || j == 'w' || j == '6')
                    value += 6;

                if (j == 'o' || j == 'z' || j == '7')
                    value += 7;

                if (j == 'f' || j == 'p' || j == '8')
                    value += 8;
                
                pValue[i] += value;
                value = 0;
                std::cout << '\n';
                clear();
            }

        }

        
        std::cin.ignore();
        std::cin.clear();

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


        for (int i = 0; i < number; ++i)
        {
            std::cout << "Value of \"" << pPhrase[i] << "\": " << pValue[i] << '\n';
            total += pValue[i];
        }

        std::cout << "Total value: " << total << "\n\nPress \'Enter\' or \'Return\' to Continue... ";

        //std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        //std::cin.clear();

        std::cout << "\n\n\nWould you like to evaluate another phrase? (Y/n):\n> ";
        std::getline(std::cin, response);

        delete[] pPhrase;
        delete[] pValue;


        if (response[0] == 'y' || response[0] == 'Y'
                || response.empty() || response[0] == ' ')
            continue;

        break;

    }

    std::cout << "Exiting...";
    try {
        //system("killall Terminal");
    } catch (std::exception& ex) {}

    std::cout << "\n\n\n";
    return 0;
}

Deja Vu! You didn't take my advice about replacing the `if` statements with an array. Are you using StackOverflow as a debugging service? Usually using a debugger is faster. — Thomas Matthews, Aug 15 '22 at 23:46
Consider stepping through the code in a debugger so you can examine the state of the program as it runs. It would help if you simplified this example to only include the code necessary to demonstrate the problem. It isn't clear which array you believe has a size of 1 or how you determined that value. — Retired Ninja, Aug 15 '22 at 23:46
Make your life easier, use `std::vector` and `std::vector::push_back`. Let the `std::vector` manage the memory and the quantity of elements. — Thomas Matthews, Aug 15 '22 at 23:53
@ThomasMatthews I remember that but I'm still learning C++ a bit and when I learn how to do it, I definitely will. Thank you for your advice. — Dev, Aug 15 '22 at 23:55
Since you don't like debuggers, add a `cout` statement before you do the dynamic memory allocation. — Thomas Matthews, Aug 15 '22 at 23:55
`std::isdigit(number)` is useless in this code. You already validated the user entered only digits, and then converted those digits to an `int` variable. What you didn't do is validate that the conversion to `int` was successful before using the `int`. — Remy Lebeau, Aug 15 '22 at 23:57
it is better to learn the best way of doing stuff in C++11++, do not learn C++03 style, so use STL classes where possible, it will save you a LOT of time — rioV8, Aug 16 '22 at 02:23
@Dev -- If you're going to tag C++17, you should be learning what facilities C++17 has to offer, instead of basically falling back and writing `C` code. *I really want to fix this problem to make it easier for my friend.* --- Believe me, the way that others have mentioned you should be doing this *is* easier than what you attempted to do. If you saw a rewrite of your code using C++17, even you would see that it is easier to understand. — PaulMcKenzie, Aug 16 '22 at 02:38
[Here is a rewrite of your code](https://godbolt.org/z/TEKjKzsTn) using what C++ has to offer. For example, note the usage of a map to map the letter with its value -- that eliminated that entire block of `if` statements. Not only that, if for some reason you want to add, remove, or change a letter or value, you just go to the map and change it, instead of wading through 8 `if` statements. — PaulMcKenzie, Aug 16 '22 at 03:09

Remy Lebeau · Answer 1 · 2022-08-16T00:18:15.080

for (int i = 0; i < number; ++i) // could replace "number" with "sizeof(pPhrase)/sizeof(pPhrase[0])"

Actually, you can't use the sizeof(array)/sizeof(array[0]) trick on a pointer to an array. It only works when you use it directly on the actual array itself. There are numerous questions on StackOverflow that explain this, including (just to name a few):

How to get the size of an array using a pointer to the first element and properties of "\0"?

Pointer to an array get size C++

getting size of array from pointer c++

In any case, one problem I do see in your code is that on each iteration of the outer while loop, you are not resetting the std::stringstream that you use to convert the user's input to the number variable. Each iteration is just pushing more and more data into the stream without removing the old data first.

Also, using std::isdigit(number) is useless. You already validated the user entered only digits, and then converted those digits to the number variable. What you didn't do is validate that the conversion to int was actually successful before using number. You must validate that the extraction of the number from the stringstream is successful, ie in case the user enters a large value that can't fit into an int.

Try this instead:

while (true)
{
    clear();

    number = 1;
    total = 0;
    value = 0;
    number_str = "";
    response = "";
    isDigit = true;

    // ADD THIS!
    ss.str("");
    ss.clear();
    //

    ...

    if (isDigit)
    {
        ss << number_str;
        if (!(ss >> number) { // <-- ADD THIS!
            number = 1;
        }
    }

    ...
}

That being said, you could just get rid of the stringstream altogether, you don't actually need it. You can extract the number value directly from std::cin itself (which you are already aware of, because you commented out that code), eg:

std::cout << "How many words to evalute? (Default: 1):\n> ";
if (!(std::cin >> number)) {
    number = 1;
    std::cin.clear();
}
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');

// no need for the `isdigit() check at all...

@OP std::vector , std::vector, std::vector, makes all this trivial — pm100, Aug 16 '22 at 00:00

Dynamically allocated array always set to size 1

1 Answers1