What's the problem with the code below ?

Question

#include <iostream>
#include <vector>
using namespace std;
int main(void)
{
    int i, s, g;
    vector<int> a;
    cin >> s;
    for(i=1;i<=s;i++)
    {
        g = s;

        if(g<10) a.push_back(g);
        else {
            vector<int> temp;
            while(g > 0)
            {

                int k = g % 10;
                g = g / 10;
                temp.push_back(g);
            }
            for(int j=temp.size();j>0;j--)
            {
                a.push_back(temp[j]);
            }
        }
    }
    cout << a[s-1] << endl;
    return 0;
}

What is wrong with the code above ? It doesn't give me the appropriate results.

The vector a is supposed to hold the values from 1, 2, 3...up to s such that a = 12345..910111213... and print to output a[s]. Ex if s=15 a=123456789101112131415 and a[15] = 2 .

If someone could tell me what's the problem

Answer 1

You have the line g = s; when I think you want g = i;. As written, for s = 5, this will be your vector a: 5 5 5 5 5 Which is not at all what you want.

[Edit] Your handling of numbers > 10 is also off. For example, what happens in your code currently for the number 12? Well, temp will be 1 0 instead of 1 2, and then this will be pushed into a as 0 1, which is again not what you want.

To fix this, think about what k is supposed to do.

Answer 2

for(int j=temp.size();j>0;j--)
{
    a.push_back(temp[j]);
}

Here the values of j include temp.size and exclude 0. Since vectors (like basically everything else with integer indices) are 0-indexed, this will access a out of bounds on the first iteration (i.e. when you access temp[temp.size()]).

Answer 3

Corrected code:

int i, s, g;
vector<int> a;
cin >> s;
for(i=1;i<=s;i++)
{
    g = i; //Why was it s?

    if(g<10) a.push_back(g);
    else {
        vector<int> temp;
        while(g > 0)
        {

            int k = g % 10;
            g = g / 10;
            temp.push_back(k); //You need to push the remainder
        }
        for(int j=temp.size()-1;j>=0;j--) //Out of bounds error
        {
            a.push_back(temp[j]);
        }
    }
}
cout << a[s-1] << endl;

return 0;

And a looks like this when s = 15 - is this what you were looking for?

a[0] = 1
a[1] = 2
a[2] = 3
a[3] = 4
a[4] = 5
a[5] = 6
a[6] = 7
a[7] = 8
a[8] = 9
a[9] = 1
a[10] = 0
a[11] = 1
a[12] = 1
a[13] = 1
a[14] = 2
a[15] = 1
a[16] = 3
a[17] = 1
a[18] = 4
a[19] = 1
a[20] = 5

Answer 4

I think there's a few problems here.

First, as MBennett said above, you should have done g = i; not g = s; to begin with.

Second, I think your inner loop also has an error, where you should be pushing back k not g as you are now.

Third, you should be doing push_front() not push_back() as you are now. Think of it this way, if you only had that loop, and had the number 162, if you push BACK (not front) every time, then it pushes 2, 6, 1, and so the sequence will have that, and not 1, 6, 2, in the order you want. Your copy after that seems OK, though there's more efficient ways of doing it.

I think that's it. Make those changes and it should function, though I haven't compiled it myself, I'm just solving in my head.