Make something smaller.

Hi.
I created something that you need to enter the Time in hour:minuts but to check if you entered a correct time I have many if's. Is there an easier way to do this? Thanks Yuri.

Code:

---

#include <iostream>

using namespace std;

string Time;

int main()
{
   
    while ( EOF )
    {
         
        system( "cls" ); 
        cout<< "Enter the Time in hour:minuts here: ";
        cin>> Time;
        cout<< endl;

        if ( Time.length() == 5 )
        {
           
            if ( Time.substr( 0, 1 ) == "0" || Time.substr( 0, 1 ) == "1" )
            {
               
                if ( Time.substr( 1, 1 ) == "0" || Time.substr( 1, 1 ) == "1" || Time.substr( 1, 1 ) == "2" ||
                    Time.substr( 1, 1 ) == "3" || Time.substr( 1, 1 ) == "4" || Time.substr( 1, 1 ) == "5" ||
                    Time.substr( 1, 1 ) == "6" || Time.substr( 1, 1 ) == "7" || Time.substr( 1, 1 ) == "8" ||
                    Time.substr( 1, 1 ) == "9" )
                {
                   
                    if ( Time.substr( 2, 1 ) == ":" )
                    {
                       
                        if ( Time.substr( 3, 1 ) == "0" || Time.substr( 3, 1 ) == "1" || Time.substr( 3, 1 ) == "2" ||
                            Time.substr( 3, 1 ) == "3" || Time.substr( 3, 1 ) == "4" || Time.substr( 3, 1 ) == "5" )
                        {
                             
                            if ( Time.substr( 4, 1 ) == "0" || Time.substr( 4, 1 ) == "1" || Time.substr( 4, 1 ) == "2" ||
                                Time.substr( 4, 1 ) == "3" || Time.substr( 4, 1 ) == "4" || Time.substr( 4, 1 ) == "5" ||
                                Time.substr( 4, 1 ) == "6" || Time.substr( 4, 1 ) == "7" || Time.substr( 4, 1 ) == "8" ||
                                Time.substr( 4, 1 ) == "9" )
                            {
                               
                                cout<< "Correct Time: "<< Time<< endl;
                                cin.get();
                                cin.get();
                            }
                           
                            else
                            {
                               
                                cout<< "Incorrect Time: "<< Time<< endl;
                                cout<< endl;
                                cout<< "ERROR: The 5th Array: "<< Time[4]<< endl;
                                cin.get();
                                cin.get();
                            }
                        }
                           
                        else
                        {
                               
                            cout<< "Incorrect Time: "<< Time<< endl;
                            cout<< endl;
                            cout<< "ERROR: The 4th Array: "<< Time[3]<< endl;
                            cin.get();
                            cin.get();
                        }
                    }
                       
                    else
                    {
                               
                        cout<< "Incorrect Time: "<< Time<< endl;
                        cout<< endl;
                        cout<< "ERROR: The 3rd Array: "<< Time[2]<< endl;
                        cin.get();
                        cin.get();
                    }
                }
               
                else
                {
                               
                    cout<< "Incorrect Time: "<< Time<< endl;
                    cout<< endl;
                    cout<< "ERROR: The 2nd Array: "<< Time[1]<< endl;
                    cin.get();
                    cin.get();
                }
            }
                   
            else if ( Time.substr( 0, 1 ) == "2" )
            {
               
                if ( Time.substr( 1, 1 ) == "0" || Time.substr( 1, 1 ) == "1" || Time.substr( 1, 1 ) == "2" ||
                    Time.substr( 1, 1 ) == "3" )
                {
               
                   
                    if ( Time.substr( 2, 1 ) == ":" )
                    {
                       
                        if ( Time.substr( 3, 1 ) == "0" || Time.substr( 3, 1 ) == "1" || Time.substr( 3, 1 ) == "2" ||
                            Time.substr( 3, 1 ) == "3" || Time.substr( 3, 1 ) == "4" || Time.substr( 3, 1 ) == "5" )
                        {
                             
                            if ( Time.substr( 4, 1 ) == "0" || Time.substr( 4, 1 ) == "1" || Time.substr( 4, 1 ) == "2" ||
                                Time.substr( 4, 1 ) == "3" || Time.substr( 4, 1 ) == "4" || Time.substr( 4, 1 ) == "5" ||
                                Time.substr( 4, 1 ) == "6" || Time.substr( 4, 1 ) == "7" || Time.substr( 4, 1 ) == "8" ||
                                Time.substr( 4, 1 ) == "9" )
                            {
                                     
                                cout<< "Correct Time: "<< Time<< endl;
                                cin.get();
                                cin.get();
                            }
                           
                            else
                            {
                               
                                cout<< "Incorrect Time: "<< Time<< endl;
                                cout<< endl;
                                cout<< "ERROR: The 5th Array: "<< Time[4]<< endl;
                                cin.get();
                                cin.get();
                            }
                        }
                           
                        else
                        {
                               
                            cout<< "Incorrect Time: "<< Time<< endl;
                            cout<< endl;
                            cout<< "ERROR: The 4th Array: "<< Time[3]<< endl;
                            cin.get();
                            cin.get();
                        }
                    }
                       
                    else
                    {
                               
                        cout<< "Incorrect Time: "<< Time<< endl;
                        cout<< endl;
                        cout<< "ERROR: The 3rd Array: "<< Time[2]<< endl;
                        cin.get();
                        cin.get();
                    }
                }
                   
                else
                {
                               
                    cout<< "Incorrect Time: "<< Time<< endl;
                    cout<< endl;
                    cout<< "ERROR: The 2nd Array: "<< Time[1]<< endl;
                    cin.get();
                    cin.get();
                }
            }
               
            else
            {
                               
                cout<< "Incorrect Time: "<< Time<< endl;
                cout<< endl;
                cout<< "ERROR: The 1st Array: "<< Time[0]<< endl;
                cin.get();
                cin.get();
            }
        }
           
        else
        {
                               
            cout<< "Incorrect Time: "<< Time<< endl;
            cout<< endl;
            cout<< "ERROR: The Size isn't correct: "<< Time.length()<< endl;
            cin.get();
            cin.get();
        }             
    }
}

---

for one thing, use the macro isdigit() from ctypes.h to check if a character is a numeric digit

Code:

---

if( isdigit( Time.substr( 1, 1 ) ) )
{
  // it is a valid digit
}

---

you could just split Time string into two integers. For this you need to make sure Time contains 4 characters.

Code:

---

int hours, minutes;
string Time = "0125";
hours = atoi(Time.substr(0,2).c_str());
minutes = atoi(Time.substr(2,2).c_str());

// now just do math on two integers
if( hours > 59)
{
  // bad number of hours
}
if(minutes > 59)
{
  // bad number of minutes
}

---

Instead of using substr you can just use operator[], so Time.substr(3, 1) would become Time[3] and you'd compare it to characters like '0' and '3' instead of strings like "0" and "3".

You can use the isdigit function (in <cctype> I guess) to check if a character is a digit to reduce the size of your checks against all 10 digits. That function takes a character, not a string, so you'd have to use Time[1] or Time[4] or whatever.

If you move the error code into a separate function, then you save all the space where you repeat the same code over and over. The only difference is the error message, so your error function can take a string with the message.

You could put your error checking at the top. For example, Check for the length, then check to see if character at position 2 is ':', then check to see if the character at position 0 is '0' or '1' (or '2' - what about military time), etc. If there is an error you can use continue to avoid nesting another level.

Quote:

---

use the macro isdigit() from ctypes.h

---

Excellent advise, except in C++ it's <cctypes>. And is it a macro?

>> except in C++ it's <cctypes>

You mean <cctype>.

Whoops, yeah, I mean <cctype>.

isdigit() is a function: see here.

Thanks, I didn't know about isdigit that helps for now, still need to test some more. Oh yeah, and shouldn't it be:

Code:

---

( isdigit( Time[1] ) )

Instead of:
( isdigit( Time.substr( 1, 1 ) ) ) ?

---

Yes. That function takes a character, not a string, so you'd have to use Time[1] or Time[4] or whatever.

Quote:

---

Originally Posted by dwks

isdigit() is a function: see here.

---

I don't have a copy of C standards bible, but from the descriptions I see with google isgigit is probably implementation defined. Some docs claim it is a function, others like this say it is a macro. One site calls it both a function in one place and a macro in another. All you have to do is read ctype.h to find out how your compiler implements it. If it says #define isdigit /*blabla*/, then its a macro. Dev-C++ defines it as an inline function that returns the result of a macro.

Thanks for all your help all. I got it like this now (please response if this is actually wrong) :

Code:

---

#include <iostream>

using namespace std;

int Dec_Time1, Dec_Time2, Dec_Time3;

string Time;

int main()
{
   
    while ( EOF )
    {
         
        system( "cls" ); 
        cout<< "Enter the Time in hour:minuts here: ";
        cin>> Time;
        Dec_Time1 = Time[0] - 48;
        Dec_Time2 = Time[1] - 48;
        Dec_Time3 = Time[3] - 48;
        cout<< endl;
           
        if ( Time.length() == 5 &&
            Dec_Time1 >= 0 && Dec_Time1 <= 1 &&
            isdigit( Time[1] ) &&
            Time[2] == ':' &&
            Dec_Time3 >= 0 && Dec_Time3 <= 5 &&
            isdigit( Time[4] ) )
        {
                               
            cout<< "Correct Time: "<< Time<< endl;
            cin.get();
            cin.get();
        }       
                                       
        else if ( Time.length() == 5 &&
                  Time[0] == '2' &&
                  Dec_Time2 >= 0 && Dec_Time2 <= 3 &&
                  Time[2] == ':' &&
                  Dec_Time3 >= 0 && Dec_Time3 <= 5 &&
                  isdigit( Time[4] ) )
        {
                               
            cout<< "Correct Time: "<< Time<< endl;
            cin.get();
            cin.get();
        }           
       
        else
        {
           
            cout<< "Incorrect Time: "<< Time<< endl;
            cout<< endl;
            cin.get();
            cin.get();
        }             
    }
}

---

There is no need to rely on the ASCII character set. Instead of subtracting 48, just use Time[0], Time[1]. etc, and check against '0', '1', ':', etc (with the quotes).

If you choose to keep the ANSCII_Dec_Code_Time variables, there is no reason for them to be global.

The rest of the code looks good. You've lost your specific error information. If that's ok with you then it makes for smaller, clearer code, but it does still change the functionality.