1

I am finding all strings from an array depending on the first letter:

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

int main(){

const std::string strArray[] =  {"an","blau","Bo","Boot","bos","da","Fee","fern","Fest","fort","je","jemand","mir","Mix",
                      "Mixer","Name","neu","od","Ort","so","Tor","Torf","Wasser"};

std::string value = "JNQ";
for_each(value.begin(), value.end(), [strArray](char c){
                  std::string const * iterator = find_if(strArray, strArray+23, [c](std::string str){
                                                    return toupper(str[0]) == c;
                                                 });
                  std::cout<<*iterator<<'\n';
          });

 return 0;
}

I get this output:

je
Name
an

Why is 'an' displayed? I am using g++ 4.5 on Ubuntu.

Improve this question

3 Answers 3

Reset to default
3

The problem with your code is that you're NOT checking iterator against the end of the array, before this line:

std::cout<<*iterator<<'\n';

which should actually be this:

if (iterator != (strArray+23))  //print only if iterator != end
     std::cout<<*iterator<<'\n';

See this. Its working now.

It doesn't print "an" anymore. :-)

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

2

iterator is not valid in 3rd case.

In this case iterator = strArray + 23 and point to element placed after array.

Look the fixed code.

Improve this answer

6 Comments

@Abhi: if (iterator != strArray + 23) { std::cout << *itrerator; } else { std::cout << "wrong"; } --> ideone.com/Vh16L
Hmm. Seems to work. Wouldn't it have been better if find_if just returned null?
@Abhi This is the way C++ algorithms are designed to work. They all return end when the item isn't found and using that approach consistently makes code much easier to maintain and understand.
@Abhi: It is not universal to return null. If you use std::vector<int>::const_iterator rather than SomeClass * for example then you can not return null. You have to return something else. The next after last is ok.
To elaborate on Mark's statement, NULL usually is a way we represent an object as "invalid", and in this case, find_if's way of marking the return value as invalid is returning strArray+23, which isn't null but it's also not an element in the array, so that's "invalid" or "not found". Just compare against strArray+23 instead of NULL and you get the same effect.
|
1

Others have already told you the iterator is invalid, so I won't repeat that. However, here is a quick type-up of a solution that should work for you. As a side-note, don't use "magic numbers" to represent your array sizes. This is error-prone because if the size of the array changes (i.e. you add another element to it later) then it is easy to forget to update the 23 to 24. Consider this solution:

static unsigned const length = sizeof( strArray );
std::string const* end = strArray+length;
std::string const * iterator = find_if(strArray, end, [c](std::string str){
                                  return toupper(str[0]) == c;
                               });
if( iterator != end ) {
    std::cout<<*iterator<<'\n';
}

Note I couldn't compile this, as I do not have a C++0x compiler, so consider this pseudo-code if nothing else.

Improve this answer

4 Comments

length is actually sizeof( strArray ) / sizeof( strArray[0] ); because otherwise it counts the size of the pointers.
@MarkB I thought about that, but since strArray is a static array, I thought sizeof(strArray) will return the number of elements in the array, not the number of bytes. Also I think it's an array of std::string s, not pointers. However, I haven't done C++ in a while so I could be mistaken. Ultimately the ideal solution is to use std::vector<std::string> anyway.
will that be efficient however?
@Abhi will what be efficient? Using std::vector? Yes. It essentially compiles down to basically the same assembler as it would if you used a regular array, like you're doing. However, efficiency isn't the only benefit. It makes your code more manageable and readable. It also cleans up a lot of boilerplate code and makes using STL algorithms more natural

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.