-1

I wrote a program that calculates cylinder and rectangle areas with user inputs. But I want improve my code. When user enter a character instead of number, it should output an error message. So I wrote some extra code but it's not working correctly.

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

using namespace std;
class area_cl
{
public:
    double height, width;
};
class rectangle : public area_cl
{
public:
    double area(double h, double w)
    {
        return h * w;
    }
};
class cylinder : public area_cl
{
private:
    double pi = 3.14;

public:
    double area(double h, double w)
    {
        return 2 * pi * (w / 2) * ((w / 2) + h);
    }
};
int main()
{
    area_cl dimension;
    rectangle obj1;
    cylinder obj2;
    bool flag = true;

    while (flag)
    {

        cout << "What is the dimensions? (Height and Width)" << endl;

        cin >> dimension.height >> dimension.width;

        if (isdigit(dimension.height) && isdigit(dimension.width))
        {
            flag = false;
        }
        else
        {
            cout << "You are not entered number,please try again." << endl;
        }
    }

    cout << "Rectangle's area is : " << obj1.area(dimension.height, dimension.width) << endl;
    cout << "Cylinder's area is : " << obj2.area(dimension.height, dimension.width) << endl;
    return 0;
}
  1. I thought of using isdigit() but my input variables must be double type and probably my code crashes for that. Are there any method in C++, like parsing in C#?

  2. I also thought about controlling input with using ASCII codes. For example if ( char variable >= 48 && char variable <= 57) but I couldn't make it work.

I would prefer to solve this problem with first option but I am totally opened to other solutions.

Thanks.

anastaciu
  • 23,467
  • 7
  • 28
  • 53
  • 1
    This might be helpful: https://stackoverflow.com/questions/24504582/how-to-test-whether-stringstream-operator-has-parsed-a-bad-type-and-skip-it And no, `isdigit()` doesn't check for `double` types, just digits, no more no less. – πάντα ῥεῖ Apr 05 '21 at 08:02

2 Answers2

1

1. I thought of using isdigit() but my input variables must be double type and probably my code crashes for that. Are there any method in C++, like parsing in C#?

isdigit just checks for digits, it's not a good fit for wahat you need. And yes there are better parsings ways in C++, not quite like C#, but at least as nice IMO (I'll get back to it ahead).


2. I also thought about controlling input with using ASCII codes. For example if ( char variable >= 48 && char variable <= 57) but I couldn't make it work.

This is also not a good option, you need doubles and that would make checking digit by digit needlessly complicated, especially beacuse this can be achieved very easily in other ways.

For future reference, when using characer comparison, you should use character literals, not all encodings have the digit codes in the same interval as ASCII:

if (char variable >= '0' && char variable <= '9')

Would be more appropriate.


I would prefer to solve this problem with first option but I am totally opened to other solutions.

The simpler and more idiomatic way I can think of is to use cin return values to check if the input is indeed valid and take measures (clear buffer and reset stream error state) if it isn't, and ask again for valid values, your code refactored would look similar to:

while (flag)
{
    cout << "What are the dimensions? (Height and Width)" << endl;
    if (cin >> dimension.height && cin >> dimension.width)
    {
        flag = false;
    }
    else
    {
        cin.clear(); // clear error flags
        cin.ignore(numeric_limits<streamsize>::max(),'\n'); // clear buffer *
        cout << "You have not entered numbers, please try again" << end;
    }
}

Or, better IMO:

cout << "What are the dimensions? (Height and Width)" << endl << ">"; // **
while (!(cin >> dimension.height && cin >> dimension.width))           
{
    cin.clear(); // clear error flags
    cin.ignore(numeric_limits<streamsize>::max(), '\n'); // clear buffer *
    cout << "You have not entered numbers, please try again" << endl << ">";
}

* - cin.ignore(numeric_limits<streamsize>::max(),'\n'); may require the inclusion of <limits> header.

This routine clears the input buffer, which will have the characters that couldn't be parsed, in case a reading error occurs.

You can read it like:

"until a newline character is found, ignore (and remove from buffer) all the characters that are there".

This will remove all the characters until a \n is found, this is the last character in the input buffer, it's added when you press Enter.


** - Notice that I changed the messaging in this second option. The input message is emmited only once at the beggining of the loop, and the error message just asks for new input if there is an input failure. The way you have it now, in case of input failure, the message flow would be:

What is the dimensions? (Height and Width)
1.9 a
What is the dimensions? (Height and Width)
You are not entered number,please try again 
_

I would prefer:

What are the dimensions? (Height and Width)
> 1.9 a
You have not entered numbers, please try again 
>_
anastaciu
  • 23,467
  • 7
  • 28
  • 53
  • I like the compressed version, however, I would argue that the cout in the while condition changes the behavior. It's also hard to read. I think a do-while loop fits better with the intend (execute this at least once). – Allan Wind Apr 05 '21 at 10:00
  • @AllanWind I gess a do..while would make things a little more clear, yes. – anastaciu Apr 05 '21 at 10:10
  • It's weird as you now handle the error before the call that triggers the error. Updated my answer with a non-goto version as it seems the easiest way to communicate what I had in mind. I used for(;;) because it's shorter to type but I still think do-while expresses the intend better. I hope that I didn't lead you astray. – Allan Wind Apr 05 '21 at 10:31
  • @AllanWind the idea is to keep the messages in order (as the OP has it), first the error message, next, the ask for input message, I reckon that one could just ask for input one time before the loop, but asking for input and next emit the error message is not a good flow. – anastaciu Apr 05 '21 at 10:35
  • I think it's good just note the change behavior that the "What are the ..." is only shown once. – Allan Wind Apr 05 '21 at 10:54
  • @AllanWind, yes I think it's a good adition, this is way more work than I was expecting ;) – anastaciu Apr 05 '21 at 11:09
  • @anastaciu Thanks for opinions and answers.And also text corrections :) – Berke Tabak Apr 05 '21 at 12:10
  • @BerkeTabak, glad to help, `cin.ignore(numeric_limits::max(), '\n');` is to clear the buffer, in case an input can't be parsed, the characters remain in the input buffer, so they must be removed. – anastaciu Apr 05 '21 at 12:17
  • @BerkeTabak added a more thorough explanation to my answer as to what it is and what it does. – anastaciu Apr 05 '21 at 12:29
0

isdigit() checks if a char is digit. When you pass it a double, it will be converted to an int. Let's say your input was 1.2 which is converted to (int) 1 which is not a digit '0' through '9'.

You can use cin.fail() to determine if it was able obtain input. If not, clear the failed state, and flush stdin with cin.ignore() of the data that could not be read. cin.bad() and cin.eof() covered in similar fashion:

#include <limits>
#include <stdlib.h>

...

retry:
    cout << "What is the dimensions? (Height and Width)" << endl;
    cin >> dimension.height >> dimension.width;
    if(cin.bad()) return EXIT_FAILURE;
    if(cin.eof()) return EXIT_SUCCESS;
    if(cin.fail()) {
        cout << "You have not entered number, please try again" << endl << ">";
        cin.clear();
        cin.ignore(numeric_limits<streamsize>::max(), '\n');
        goto retry;
    }

or the non-goto variant (for discussion with @anastaciu):

#include <limits>
#include <stdlib.h>

...

for(;;) {
    cout << "What is the dimensions? (Height and Width)" << endl;
    cin >> dimension.height >> dimension.width;
    if(cin.bad()) return EXIT_FAILURE;
    else if(cin.eof()) return EXIT_SUCCESS;
    else if(cin.fail()) {
        cout << "You have not entered number, please try again" << endl << ">";
        cin.clear();
        cin.ignore(numeric_limits<streamsize>::max(), '\n');
    } else
        break;
}

Another option is to read a line of text and parse that. It would allow you to provide a better error handling than losing all user input.

Allan Wind
  • 23,068
  • 5
  • 28
  • 38
  • Not my downvote. but many users are not very fond of goto, that may very well be the reason. You can find many posts decrying its usage, like this one https://stackoverflow.com/q/3517726/6865932 – anastaciu Apr 05 '21 at 08:48
  • Seems cleaner than the alternative `bool flag=true; while(flag) { if() flag=false; else .. }`. Reasonable people can disagree. I will delete the answer if it's not helpful. – Allan Wind Apr 05 '21 at 09:53
  • I personaly have no problem with it, I was just giving you a heads up as to maybe why the DV since there is no feedback from the downvoter, and seemingly nothing wrong with the answer. No deletion needed from my point of view. – anastaciu Apr 05 '21 at 09:56
  • No worries, buddy. – Allan Wind Apr 05 '21 at 10:03
  • There is absolutely nothing wrong with `goto`. Maybe a bulk of people using C++ don't know what an unconditional `jmp` instruction is, but that doesn't equate to there being anything wrong with `goto`. My only comment would be to check `cin.bad()` and `cin.eof()` to cover rare but unrecoverable stream issues and to allow the user to cancel input by entering a manual `EOF`, e.g. `ctrl + d` (`ctrl + z` on windows) – David C. Rankin Apr 05 '21 at 10:43
  • @DavidC.Rankin Updated as suggested. – Allan Wind Apr 05 '21 at 10:47
  • That covers it. Only other thought is this. If the user cancels with a manual `EOF`, that technically isn't an error. You either have control return to the previous scope or return `0` as you do. On the other hand, if `badbit` is set, that is a true error and the return value should reflect the unrecoverable error. What you have is fine for the purpose of the question, just put the other considerations in the toolbox for use where appropriate. – David C. Rankin Apr 05 '21 at 11:35
  • @AllanWind Thanks for answering. But i am very new in c++ and is `cin.ignore(numeric_limits::max(), '\n');` line necessarily? I mean why we use that line? – Berke Tabak Apr 05 '21 at 12:15
  • It's some C++ magic that says remove everything up to the next newline. If you don't, your cin will fail again for the same reason it failed the first time (i.e. if enter "a" instead of a number). – Allan Wind Apr 05 '21 at 13:00