Search code examples
stringc++11limitgetline

Is there any particular way to use getline for string data type in C++ for reading in a string of say 540 characters?


I have to read in this string : efBZw WH EDC EVOh qjfzJ oXkXL QUHXWMl kXRSIyGHb TxQkBWhPI yPTJEZ KyYfFQR ZQvYBPUmZ bkuKbRBWW mjJ WRgIBFNM ojGP XbkJNUhXF SeZVBZ SooFP XXE VIebym gOXP pDXRQ

No output for the given string.

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

    using namespace std;

    int isPiSong(string str,deque<int> numbers){
        int noOfwords = 0, noOfletters = 0, ctr = 0;
        int flag = 1;
        for(std::string::iterator it = str.begin(); it != str.end(); ++it) {

            if(*it != ' ' && flag){
                if(isalpha(*it))
                noOfletters++;
            }
            else{
                if(noOfletters != numbers[ctr++])
                    flag = 0;
                noOfletters = 0;
            }

        }
        return flag;
    }

    int main()
    {
        deque<int> numbers;
              // 31415926535897932384626433833
        numbers.push_back(3);
        numbers.push_back(1);
        numbers.push_back(4);
        numbers.push_back(1);
        numbers.push_back(5);
        numbers.push_back(9);
        numbers.push_back(2);
        numbers.push_back(6);
        numbers.push_back(5);
        numbers.push_back(3);
        numbers.push_back(5);
        numbers.push_back(8);
        numbers.push_back(9);
        numbers.push_back(7);
        numbers.push_back(9);
        numbers.push_back(3);
        numbers.push_back(2);
        numbers.push_back(3);
        numbers.push_back(8);
        numbers.push_back(4);
        numbers.push_back(6);
        numbers.push_back(2);
        numbers.push_back(6);
        numbers.push_back(4);
        numbers.push_back(3);
        numbers.push_back(3);
        numbers.push_back(8);
        numbers.push_back(3);
        numbers.push_back(3);

        int T, flag;
        cin>>T;

        string* pisong = new string[T];

        for(int i = 0; i < T; i++){
           getline(cin>>ws, pisong[i]); //cin >> ws gets rid of leading whitespace 
                                       //first so that getline won't think that it's already                         //reached the end of the line
                                      //It doesn't seem to take the above input string
        }

        for(int i = 0; i < T; i++){
            flag = isPiSong(pisong[i],numbers);
            if(flag){
                cout<<"It's a pi song."<<endl;
            }else{
                cout<<"It's not a pi song."<<endl;
            }
        }
        delete [] pisong;
        return 0;
    }

Solution

  • You want to install valgrind and use it. It reveals immediately this problem:

    ==6962== Conditional jump or move depends on uninitialised value(s)
    ==6962==    at 0x40117D: isPiSong(std::string, std::deque<int, std::allocator<int> >) (stackoverflow-31686882.cpp:19)
    ==6962==    by 0x401670: main (stackoverflow-31686882.cpp:77)
    ==6962== 
    ==6962== Use of uninitialised value of size 8
    ==6962==    at 0x401173: isPiSong(std::string, std::deque<int, std::allocator<int> >) (stackoverflow-31686882.cpp:19)
    ==6962==    by 0x401670: main (stackoverflow-31686882.cpp:77)
    ==6962== 
    ==6962== Invalid read of size 4
    ==6962==    at 0x401173: isPiSong(std::string, std::deque<int, std::allocator<int> >) (stackoverflow-31686882.cpp:19)
    ==6962==    by 0x401670: main (stackoverflow-31686882.cpp:77)
    ==6962==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
    ==6962== 
    ==6962== 
    ==6962== Process terminating with default action of signal 11 (SIGSEGV)
    ==6962==  Access not within mapped region at address 0x0
    ==6962==    at 0x401173: isPiSong(std::string, std::deque<int, std::allocator<int> >) (stackoverflow-31686882.cpp:19)
    ==6962==    by 0x401670: main (stackoverflow-31686882.cpp:77)
    

    Line 19 is this one:

    if(noOfletters != numbers[ctr++])
    

    ctr is incremented for every single space. Once the numbers deque is exhausted (i.e. ctr > length) you get illegal accesses in the deque. So, either you have to reset ctr to zero at some point, or you have to check for ctr to be not too big.

    Additionally there is much room for improvement.

    Once flag is set to zero, it can't change any longer and so at some point, you will return false. So, why not just return immediately? So Instead of flag = 0, just return 0.

    You first count the number of letters, then when you reach the next space, you check it. What if you give this string: "abc a abcd xxxxxxxxxxxxxxxx"? Wait, let me test it:

    It's a pi song. [abc a abcd xxxxxxxxxxxxxxxx]
    

    Only if I add a space after the last x, the program correctly detects the error:

    It's not a pi song. [abc a abcd xxxxxxxxxxxxxxxx ]
    

    Solution: Once you detect the first isalpha character, set some n = numbers[ctr++] - 1 (after checking, that there are numbers left). Then for each following isalpha do n-- and check if it was zero before (ie "if (n--) {...}". If it was -> return 0. If not, go on until space. The space test will then check n to having reached zero (which means, that exactly numbers[ctr++] isalphas have been skipped). If not -> return 0.

    Finally you have to decide what to do, if the numbers deque is exhausted but input follows.