Vector subscript out of range - vector of structures

Question

I have the following code. It's supposed to count the number of repetitions of the given letter in a given file. However, when i try to run this i get the Vector subscript out of range. Other people with the same error were trying to access undefined parts of it, but this doesn't seem to be an issue here i think.

struct letters
{
    char letter;
    int repetitions=0;
};


void howManyTimes(const string &output)
{
    ifstream file(output);
    vector <letters> alphabet;

    for (int i = 0; i < 'z' - 'a' + 1; i++)
    {
        alphabet[i].letter = 'a' + i;
    }

    string line;

    while (file.eof() == 0)
    {
        getline(file, line);
        for (char c : line)
        {
            if(c  >= 'a' && c <= 'z')
                alphabet[c - 'a'].repetitions++;
            else if (c >= 'A' && c >= 'Z')
                alphabet[c - 'A'].repetitions++;
        }
    }
    cout << alphabet[10].repetitions;
}

The vector needs to be initialized befor you use it. Otherwise use `push_back('a' + i)` instead of the `operator[]` to assign values. — πάντα ῥεῖ, Nov 17 '16 at 15:55
You really should be using a `std::map` if you want to find the number of occurrences of unique items. — NathanOliver, Nov 17 '16 at 15:56
Unrelated to the problem you ask about, but you should really take some time to read [Why is iostream::eof inside a loop condition considered wrong?](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong). — Some programmer dude, Nov 17 '16 at 15:59
_"i get the Vector subscript out of range. Other people with the same error were trying to access undefined parts of it, but this doesn't seem to be an issue here i think."_ Why do you think that? When your computer is telling you that it is literally that. — Lightness Races in Orbit, Nov 17 '16 at 16:37

score 2 · Accepted Answer · answered Nov 17 '16 at 15:56

vector <letters> alphabet;  // (1)

for (int i = 0; i < 'z' - 'a' + 1; i++)
{
    alphabet[i].letter = 'a' + i;  // (2)
}

(1) creates an empty vector.

Inside the for loop in (2) you try to access items using the index i in an empty vector, so clearly your index is out of range.

You first have to populate the vector with some data, then you can access this data.

If you want to add new items to the vector, you can use vector::push_back (which is probably what you meant in (2)).

Sergey · Answer 2 · 2016-11-18T01:25:28.357

0

Other people with the same error were trying to access undefined parts of it, but this doesn't seem to be an issue here I think.

It is definitely an issue here:

vector <letters> alphabet; // Now size of the vector is 0.

for (int i = 0; i < 'z' - 'a' + 1; i++)
{
    // You are trying to access elements 0, 1, 2, ..., which do not exist.
    alphabet[i].letter = 'a' + i;
}

The simplest solution is to construct your vector with appropriate size:

vector <letters> alphabet('z' - 'a' + 1);

edited Nov 18 '16 at 01:25

answered Nov 17 '16 at 15:56

Sergey

7,985
4
48
80

It should be noted that `'z' - 'a'` doesn't work on systems. It will work on systems using ASCII encoding (which is a vast majority of all systems), but it's not truly portable. – Some programmer dude Nov 17 '16 at 16:00
It's working! But it had to be `'z' - 'a' + 1` And i had this `(c >= 'A' && c >= 'Z')` If wrong. Sign should be `c < 'Z'` – Jarek N Nov 17 '16 at 20:41

Xirema · Answer 3 · 2016-11-17T16:41:25.197

I don't see the part of your code where alphabet is expanded to accommodate the objects you plan to store in it. std::vector only resizes itself when you use the push_back, insert, emplace or other similar methods; it doesn't do so when accessing directly using the operator[] method.

At any rate, for a task like this, I'm not sure you'd want to use a vector, when a std::map<char, int64_t> would probably be a lot cleaner, and would let you preserve the syntax you're trying to use without tons of extra maintenance.

void howManyTimes(const string &output)
{
    ifstream file(output);

    map<char, int64_t> alphabet;

    string line;

    while (getline(file, line))
    {
        for (char c : line)
        {
            if(c  >= 'a' && c <= 'z')
                alphabet[c - 'a']++;
            else if (c >= 'A' && c >= 'Z')
                alphabet[c - 'A']++;

        }
    }
    cout << alphabet[10];

}

You should at least note that the loop condition is wrong, if you're going to reproduce it — Lightness Races in Orbit, Nov 17 '16 at 16:38
@LightnessRacesinOrbit I honestly did not notice that. I copied their code and made changes to the stuff I was focused on. I'll fix it up. — Xirema, Nov 17 '16 at 16:41

Kenneth McKanders · Answer 4 · 2016-11-18T13:40:30.687

This program is really powerful for what the question is asking (makes me wonder how my students felt when I gave it as a homework assignment :) ). Do you have to use a struct? Let's assume not, and also assume we know the size of the alphabet, and that 'a' is the first letter and 'z' is the last letter:

vector<int> repetitions(26, 0);
char nextCharacter;
while( !file.eof() )
{
    cin >> nextCharacter;
    nextCharacter = tolower(nextCharacter);
    if(nextCharacter >= 'a' && nextCharacter <= 'z')
    {
        repetitions[nextCharacter - 'a']++;
    }
}

To check for a letter:

cin >> letterToQuery;
cout <<"The amount of occurrences of " << letterToQuery <<" is ";
cout << repetitions[tolower(letterToQuery) - 'a'] << endl;

If you don't know the size of your alphabet, the code changes to:

vector<int> repetitions('last_alphabet' - 'a' + 1, 0);
...
if(nextCharacter >= 'a' && nextCharacter <= 'last_letter')

And finally, if you DO have to use that struct, your code changes to:

struct Letter
{
    char letter;
    int repetitions=0;
};

vector<Letter> alphabet;
letter temp;
for(int i = 0; i < 'last_alphabet' - 'a' + 1; ++i)
{
    temp.letter = 'a' + i;
    alphabet.push_back(temp);
}

// then everything else is a similar structure
char nextCharacter;
while( !file.eof() )
{
    cin >> nextCharacter;
    nextCharacter = tolower(nextCharacter);
    if(nextCharacter >= 'a' && nextCharacter <= 'last_alphabet')
    {
        alphabet[nextCharacter - 'a'].repetitions++;
    }
}

To check for a letter:

cin >> letterToQuery;
cout <<"The amount of occurrences of " << letterToQuery <<" is ";
cout << alphabet[tolower(letterToQuery) - 'a'].repetitions << endl;

Notice, if you replace 'last_alphabet' with 'z', you get the current alphabet.

Vector subscript out of range - vector of structures

4 Answers4