Why is this function returning an empty vector?

Question

In the code below I am trying to return an array containing the longest strings of the inputArray. However, when I use it the array outputted is empty.

vector<string> solution(vector<string> inputArray) {
    int highestSize, add;
    vector<string> newArray{};

    for (int i = 0; i < inputArray.size(); ++i) {
        highestSize = max(int(inputArray[i].length()), highestSize);
    }

    for (int i = 0; i < inputArray.size(); ++i) {
        if (inputArray[i].length() == highestSize) {
            newArray[add] = inputArray[i];
            ++add;
        }
    }

    return newArray;
}

1. You haven't initialized `highestSize, add` to `0`. 2. you haven't allocated space in `newArray` to use operator[]. You should use `push_back` instead. — wohlstad, Aug 06 '22 at 05:49
A side note: passing `inputArray` by `const&` will be more efficient. — wohlstad, Aug 06 '22 at 05:50
I recommend to [edit] your question; to clarify and to get closer to [ask] and to [mre]. — Yunnosch, Aug 06 '22 at 05:52
@wohlstad Thanks, your push_back suggestion worked! I'm still learning so didn't know of this function until now. — Justin Zhu, Aug 06 '22 at 05:54
Note, you may find it confusing to use the word "array" in naming vectors. — Chris, Aug 06 '22 at 06:11
Recommendation: Don't waste your time competing on judging sites before learning the language. — user4581301, Aug 06 '22 at 06:27

wohlstad · Accepted Answer · 2022-08-06T06:05:28.563

There are several issues in your code:

highestSize and add are not initialized. In C++ variables are not default initialized to 0 as you might have expected.
newArray is default constructed to have 0 elements. In this case you cannot use operator[] the way you did. operator[] can access only elements that were allocated. You can use push_back to add to the vector. If your code was changed so that you knew in advance how many entries you will need in newArray, you could also resize the vector and then use operator[] to access the elements. There are some more options like resereving capacity and using push_back, you can see more info here.
It's better to pass inputArray by const& to avoid an unneeded copy.

A fixed version:

#include <string>
#include <vector>

std::vector<std::string> solution(std::vector<std::string> const & inputArray) {
    int highestSize{ 0 };
    std::vector<std::string> newArray{};
    for (int i = 0; i < inputArray.size(); ++i)
    {
        highestSize = std::max(int(inputArray[i].length()), highestSize);
    }
    for (int i = 0; i < inputArray.size(); ++i)
    {
        if (inputArray[i].length() == highestSize)
        {
            newArray.push_back(inputArray[i]);
        }
    }
    return newArray;
}

A side note: it's better to avoid using namespace std - see here Why is "using namespace std;" considered bad practice?.

Why is this function returning an empty vector?

1 Answers1