What's wrong with my code?

[MultiMedia]

The following is a function that tests whether the user entered the right input. In this case, I am testing for an integer and the function works fine. However, when the user enters a number like f2 or a2, it breaks (accessing unknown memory location), but how come?

int getvalue = 0;
	bool legal = false;
	cin >> text;

	while (!legal )
	{
		for (unsigned int i = 0; i < text.size(); ++i)
		{
			if (!(isdigit(text[i])))
			{
				i = 0;
				text = "";
				cout << "Incorrect input, try again: ";
				cin.clear();
				cin.ignore(256, '\n');
				cin >> text;
			}
			else if ((i == text.size() - 1) && (isdigit(text[i])))
			{
				legal = true;
			}
		}
	}

	if (legal)
	{
		getvalue = stoi(text);
		return (getvalue );
	}
	else
	{
		return 0;
	}

[Thomas1965]

I would do it like this:

#include <iostream>
#include <string>
#include <ctype.h>

using namespace std;

bool isInt (const string& s);

int main ()
{
  bool valid = false;
  string input;

  while (!valid)
  {
    cout << "\nEnter your number: ";
    cin >> input;
    if (isInt (input))
      valid = true;
    else
      cout << "\nInvalid input. Try again ";
  }
  int num = stoi (input);
  cout << "The number you entered is: " << num << "\n\n";

  system ("pause"); // remove if you don't use Visual Studio
  return 0;
}

bool isInt (const string& s)
{
  for (const char ch : s)
  {
    if (!isdigit (ch))
      return false;
  }
  return true;
}

[jlb]

But don't forget that with stoi() even if all of the characters are digits doesn't mean that stoi() will succeed. The stoi() function will fail if the number is out of bounds for the type.

Also what happens if the user entered something like: "123 ABCD 234"? Remember only 123 will be processed by your program, everything else is still in the input buffer.

Perhaps you might consider exceptions?

#include <iostream>
#include <string>
#include <ctype.h>
#include <exception>

using namespace std;

void isInt(const string& s);

int main()
{
    bool valid = false;
    string input;
    int num;

    while(!valid)
    {
        cout << "\nEnter your number: ";
        getline(cin, input); // Use getline to insure you process the entire line.

        try
        {
            isInt(input);
            num = stoi(input);
            valid = true;
        }
        catch(const std::invalid_argument& e)
        {
            cerr << "\nInvalid input. Try again\n";
        }
        catch(const std::out_of_range& e)
        {
            cerr << "\nInput out of range. Try again\n";
        }

    }

    cout << "The number you entered is: " << num << "\n\n";

    system("pause");  // remove if you don't use Visual Studio
    return 0;
}

void isInt(const string& s)
{
    for(const char ch : s)
    {
        if(!isdigit(ch))
            throw(std::invalid_argument("has_non_digit"));
    }
}

[Thomas1965]

You are right. Somehow I missed possible spaces in the input or an overflow. Exceptions are probably the safest option, but maybe too advanced for a beginner.

[jlb]

but maybe too advanced for a beginner.

Possibly, depending on how the language is being taught. Dr. Stroustrup teaches exceptions early, Chapter 5 in "Programming Principles and Practice Using C++", and recommends their use over return values in most circumstances.

IMO when dealing with a function that throws an exception to indicate an error as do the stoX() series of functions it is better to use the exceptions to your advantage instead of fighting them.

If you're not ready for exceptions I recommend using stringstreams instead, since by default most streams don't throw an exception on error.

[MultiMedia]

Thank you both, that was very helpful. I taught myself how to use exceptions.