C++ isMember function crash

Question

I am making a program that will test for a string's existence in a string array. I have made a simple function to do so:

bool isMemberOfArrayStr(std::string str, std::string array[256]){
  for(int i=0;array->length()<10;i++){
    if(array[i]==str){
      ret=true;//A global variable that will be reset to false after the function call
    }
  }
  return ret;
}

The definition of the function causes no errors, but the call:

if(neighbors[2] == isMemberOfArrayStr(neighbors[2], validTokens))
{
  std::cout <<"true"
}

Causes the runtime error "Segmentation fault: 11". I'm not sure what the problem is and help would be nice.

I advice to you include in you code `using namespace std;` to avoid the `std::` constantly. — Victor Sigler, Feb 05 '14 at 16:39
@Vkt0rS. Never recommend that. [`using namespace std` is a bad programming practice.](http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) — David G, Feb 05 '14 at 16:42
@0x499602D2 - it's bad practice is a header, but not so bad in a .cpp — Sean, Feb 05 '14 at 16:44
`neighbors[2]` is a `std::string` (since it can be passed into `isMemberOfArrayStr` as the first argument. Why then the checking for `neighbors[2] == isMemberOfArrayStr(neighbors[2], validTokens)`. That is essentially comparing `std::string == bool`. — Chad, Feb 05 '14 at 16:45
Disregarding the fact that the magic array-becomes-pointer trap has caught you by surprise, why would you loop for as long as the length of the array is less than 10? Do you expect its length to change while you're looking at it? — molbdnilo, Feb 05 '14 at 16:49

score 1 · Answer 1 · answered Feb 05 '14 at 16:39

1

array->length();

Is not a length of array. This is a length of first std::string in array. Consider using std::vector of std::string.

Unless the order is important in the container, you should better choose a std::set (multiset).

answered Feb 05 '14 at 16:39

αλεχολυτ

4,792
1
35
71

Would using (sizeof(array)/sizeof(*array) – bakman329 Feb 05 '14 at 16:50
@bakman329 if use it inside function, answer "no". Because 'array' in this case is a simple pointer to std::string. – αλεχολυτ Feb 05 '14 at 16:52
@0x499602D2 Only vector alone certainly is not enough. I just think it is better to use more high-level constructs to avoid random errors (confusion between pointers and arrays). – αλεχολυτ Feb 05 '14 at 16:56

Jarod42 · Accepted Answer · 2014-02-05T17:26:54.223

1

bool isMemberOfArrayStr(std::string str, std::string array[256])

decays to

bool isMemberOfArrayStr(std::string str, std::string* array)

The correct prototype is

bool isMemberOfArrayStr(std::string str, std::string (&array)[256])

but you may use std::array or std::vector which have better (more intuitive) syntax.

Then array->length() is not 256, but the length of the first string of the array.

Your function may be rewritten (C++11):

bool isMemberOfArrayStr(const std::string& str, std::string (&array)[256]) {
    ret = std::find(std::begin(array), std::end(array), str) != std::end(array);
    return ret;
}

C++03:

bool isMemberOfArrayStr(const std::string& str, std::string (&array)[256]) {
    ret = std::find(array, array + 256, str) != array + 256;
    return ret;
}

And I think you should remove ret from this function anyway.

edited Feb 05 '14 at 17:26

answered Feb 05 '14 at 16:45

Jarod42

203,559
14
181
302

This looks like an improvement, but I may be doing something wrong. I still get several errors: non-member function cannot have 'const' qualifier, no member named 'begin' or 'end' in namespace 'std', no matching function for call to 'isMemberOfArrayStr' ...isMemberOfArrayStr(neighbors[2],validTokens)){ – bakman329 Feb 05 '14 at 17:08
@bakman329: Added C++03 version. previous was C++11. (And I though that it was a method). – Jarod42 Feb 05 '14 at 17:28

score 0 · Answer 3 · answered Feb 05 '14 at 16:40

bool isMemberOfArrayStr(std::string str, std::string array[256]){
  for(int i=0;array->length()<10;i++){
                     ^^^^^^^^^^
    if(array[i]==str){
      ret=true;//A global variable that will be reset to false after the function call
    }
  }
  return ret;
}

you are comparing constant value. You are executing for loop only if array->length()<10 and then doing it infinitely many times. Also

array->length()

is a length of first element (if any!) in array.

C++ isMember function crash

3 Answers3