Ayush Yadav
Ayush Yadav

Reputation: 1

Why is this std::vector giving runtime error?

vector<int> matchingStrings(vector<string> s, vector<string> q) {
        
        vector<int> res;
        map<string, int> mp;
        for(int i = 0 ; i < s.size();i++){
            mp[s[i]]++;
        }
        
        for(int i = 0 ; i < q.size();i++){
                res[i] = mp[q[i]]; 
        }
        return res;
}

Upvotes: 0

Views: 219

Answers (2)

user12002570
user12002570

Reputation: 1

The problem is you have an empty vector and by writing:

res[i];

you're trying to access its ith element which doesn't exist. You can solve this by using push_back on res as follows:

res.push_back(mp[q[i]]);//you could use emplace_back here instead

Also there are 2 advice that i would like to give here:

  1. Use .at() instead of [] on the std::map whenever you don't want to create unintentional elements.

  2. In this case you can also use emplace_back instead of push_back.

Taking point 2 of the advice into consideration, you could write the above suggested statement as:

res.emplace_back(mp[q[i]]); //use .at() only if you don't want to create/add elements into the map and just want to read

So the modified code would look like:

vector<int> matchingStrings(vector<string> s, vector<string> q) {
        
        vector<int> res;//res is an empty vector
        map<string, int> mp;
        for(int i = 0 ; i < s.size();i++){
            mp[s[i]]++;
        }
        
        for(int i = 0 ; i < q.size();i++){
                res.emplace_back(mp[q[i]]); //used emplace_back
        }
        return res;
}

Alternative Solution:

Create res to be of a particular size.

vector<int> res(q.size());//res has size equal to q's size

Now you can use res[i]; safely and there is no need to use push_back and emplace_back.

So the modified code would look like:

vector<int> matchingStrings(vector<string> s, vector<string> q) {
        
        vector<int> res(q.size());//res has size q
        map<string, int> mp;
        for(int i = 0 ; i < s.size();i++){
            mp[s[i]]++;
        }
        
        for(int i = 0 ; i < q.size();i++){
                res[i] = mp[q[i]]; //res[i] is fine now 
        }
        return res;
}

Upvotes: 2

IkarusDeveloper
IkarusDeveloper

Reputation: 408

In additional to the answer of @Anoop Rana, i think you could also reduce the effort of coding the given function by using std::count (Since c++17).

std::vector<int> matchingStrings(const std::vector<string>& s, const std::vector<string>& q) {
    std::vector<int> ret{};
    ret.reserve(q.size());
    for(auto& str: q)
        ret.emplace_back(std::count(s.begin(), s.end(), str));
    return ret;
}

Note : Using const std::vector<string>& you can avoid copying values while using matchingStrings

Upvotes: 1

Related Questions