0

I tried to find the index of an element in an array....i managed to make it work with ints using the following function:

int *getIndexOfInt(int *arr, int size, int check) {
  int *result;
  int *k;
  int count = 0;
  for (int i = 0; i <= size - 1; i++) {
    if (arr[i] == check) {
      k[count] = i;
      count++;
    }

  }
  if (count > 0) {
    *result = *k;
    return result;
  } else
    cout << "Not Found";
}

However when i tried this for strings it just gave me errors (program exited with status 11) or infinite loops:

int *getIndexOfString(string *arr, int size, string check) {
    int *result;
    int *k;
    int count = 0;
    for (int i = 0; i <= size - 1; i++) {

         if (arr[i] == check) {

            k[count] = i;
            count++;
          }

    }

    if (count > 0) {

        *result = *k;
        return result;
  } 
   else cout << "Not Found";
}

Can you please tell me why and maybe help me fix the errors?

EDIT: The result variable is the array which is then used in the main function,it contains the indexes where the string was found in the given array. The k variable is just an array in which values are stored before being added into the result. The arr is the given string array the size is the givven size and the check is the string which the code will search for.

8
  • first glance show that your first function doesn't work. int* k and int* result is never initialized. Commented Oct 21, 2013 at 17:37
  • then i have another question..why does the first one work? Commented Oct 21, 2013 at 17:38
  • 1
    If this is not for training purposes I would reccomend to use std::find. Commented Oct 21, 2013 at 17:40
  • I'd say its working by luck :o ... You do not know where "k" is pointing to and therefore where k[count] is in memory... undefined, so it "could" in theory work, but as soon as you change some code and it is re-compiled it could stop working Commented Oct 21, 2013 at 17:41
  • 1
    @Nobody And std::vector. If you're not using std::find, you're not using an important part of C++; if you're not using std::vector, you're using parts of C++ which are broken (because of C compatibility). Commented Oct 21, 2013 at 17:43

4 Answers 4

4

First of all, you are accessing to uninitialized memory. The weird thing is, that your first code works. However, it's probably compiler-specific (these things happens in C++ a lot).

Local variables are usually allocated on stack and C++ doesn't guarantee you any default value. Therefore, one possible explanation is that there was (on the same memory address, where your pointer is saved) another, valid, pointer. Now, when you created this local variable, it just got this "old" address and so it's accessing to some previously allocated memory. Just don't care about it at the moment and even if it works, believe us - you shouldn't rely on this. :-)

Another problem is with that returned value. How you'd use that, when you don't know size of that array? You should return something like std::vector<>, some structure or something like that. Not just pointer to an array of unknown length!

Result: Your code is way too complicated than it could be. See better solution:

#include <iostream>
#include <string>
#include <vector>

std::vector<int> getIndexes(std::vector<std::string> &input, std::string searched) {
    std::vector<int> result;

    for (int i = 0; i < input.size(); i++) {
        if (input[i] == searched) {
            result.push_back(i);
        }
    }

    return result;
}

int main(int argc, char *argv[]) {
    std::vector<std::string> greetings;
    greetings.push_back("hello");
    greetings.push_back("hi");
    greetings.push_back("bye");
    greetings.push_back("hi");
    greetings.push_back("hello");
    greetings.push_back("bye");

    std::vector<int> indexes = getIndexes(greetings, "hi");

    for (int i = 0; i < indexes.size(); i++) {
        std::cout << indexes[i] << std::endl;
    }

    return 0;
}
Sign up to request clarification or add additional context in comments.

1 Comment

I tested it ..and it works fine...the only reason i did not used the vector library..is because i have never worked with it...thank you for your response.
2

Others have suggested it, and here I place it for reference.

You can use the Standard Library. In particular the algorithm std::find:

#include<vector>
#include<string>
#include<iostream>
#include<algorithm>

int main() {
  std::vector<std::string> words = {"one", "two", "three", "four", "five"};
  size_t index = std::distance(words.begin(),
                               std::find(words.begin(), words.end(), "three"));
  std::cout<<"Index: "<<index<<std::endl;
}

compiled as (GCC 4.8.1 OS X 10.7.4):

g++ indices-in-array.cpp -std=c++11

output:

Index: 2

2 Comments

like i said..this was for training purposes ... so i had to use something more complicated than find() :)
Sure --- like I said: it is here for reference. Other people search for Stackoverflow answers and they may need the actual code using std::find.
1

Your main problem is that you're not initializing your result pointer to a valid array. Really though you should return a vector of indices rather than a pointer, this way the caller knows the size and doesn't have to manage the memory manually. Something like this:

#include <iostream>
#include <vector>
#include <string>
#include <algorithm>
#include <iterator>

using namespace std;

vector<int> getIndicesOfString(const vector<string>& in, const string& check) {
    vector<int> ret;
    for (auto it = cbegin(in); it != cend(in); ++it) {
        if (*it == check) ret.push_back(distance(cbegin(in), it));
    }
    return ret;
}

int main() {
    auto v = vector<string>{"red", "orange", "yellow", "green", "blue", "indigo", "violet", "red"};
    auto indices = getIndicesOfString(v, "red");
    copy(cbegin(indices), cend(indices), ostream_iterator<int>(cout, ", "));
}

Comments

0

Edited for vector use:

std::vector<int> getIndexOfString(string *arr, int size, string check)
{
  std::vector<int> iVect;
  for (int i = 0; i <= size - 1; i++)
  {
    if (arr[i] == check)
    {
      iVect.push_back (i);
      cout << "Found at " << i << endl;
    }
  }

  if (iVect.empty)
     cout << "not found" << endl;

  return iVect;
}

Your function has too many un-initialized pointers. Are you really wanting to return an index or a pointer? You are also missing the return value for the failed case.

3 Comments

my code is supposed to return all of the indexes...for example if the givven array is "a", "b","a" it will output the result in an array where at [0] we will have 1 and at [1] we will have 3 (if we search for a)
@SpiderLinked My mistake, I missed that part of your requirement. In that case I would also strongly recommend that you return a vector (or pass one in). Edited for a vector example....
Thank you for your response...it is also good...(i did not marked as the answer because i do not need any output from the function, while this function should be in an external header file)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.