文字列検索について

質問

下記のようなC++プログラムにおいて、Good Morning! の「r」以後が、
検索にひっかからないで、-1を返します。
何故なのか、どうぞよろしくお願いします。

--------------------------------------------------------
#include <iostream>
#include <cstring>
using namespace std;

int flag = 0; // 該当文字があったかどうかのフラグ

int strch_idx(const char* s, char c){
int temp;
cout << strlen(s) << endl;
for(int i=0; i<(signed)strlen(s); i++){
if(*s == c){
temp=i+1; // 配列のインデックスは０オリジンだが、インデックスは1からだから１+する
flag = 1;
break;
}
s++;
}
if(flag == 1) return temp;
else return -1;
}

int main(){
const char* s = "Good Morning!";
int idx = strch_idx(s, 'i');
cout << "検索文字のインデックス：" << idx << endl;

return 0;
}

回答No.3

hanabutako · ベストアンサー

#1の回答で既に答えが出ていますが、補足します。
for(int i=0; i<(signed)strlen(s); i++){
...
}
というコードなのに、ループ内でs++;しているのが問題です。ループを回るたびにstrlen(s)が評価され、s++されているので当然ループを回るたびに値がstrlenの結果が1ずつ減り、全部の文字列を探索しません。

s++というコードを書きたいなら、
while (*s != '\0') {
...
s++;
}
というコードにしてはいかがでしょうか。

あるいは、sを変更しないようにします。
...毎回、strlenを呼んでいるのが無駄だと思いますが。
for(size_t i=0; i<strlen(s); i++){
if (s[i] == c) {
...
}
...
}

しかしながら、文字の検索をするとしたら、プログラミングの勉強でもない限り、#2の回答のようにするか、strchrを使うかだと思います。
#include <string.h>
int strch_idx(const char* s, char c) {
char* p = strchr(s, c);
return p == NULL ? -1 : p - s;
}

あと、これも何を言っているのかわかりませんが。
// 配列のインデックスは０オリジンだが、インデックスは1からだから１+する
このルール通りにやるとしたら、#2の回答も自分のstrchrのプログラムも文字を発見した時の返り値を+1したほうがよいでしょうね。インデックスを1始まりとしてC言語/C++のプログラムを書くのはやめたほうがよいと思いますが。

お礼

ご回答、どうもありがとうございました。

補足

長文のご説明、どうも有難うございました。
インデックスを１オリジンにすることは、わかってはいるのですが、極力避けたいと思います。

回答No.2

επιστημη（@episteme）

[1] std::string つかえばいいのに
int strch_idx(const std::string& s, char c) {
std::string::size_type p = s.find(c);
return p == std::string::npos ? -1 : p;
}

[2] あるいは:
int strch_idx(const char* s, char c) {
const char* p = std::find(s, s+strlen(s), c);
return p == s+strlen(s) ? -1 : p - s;
}

お礼

ご回答、どうもありがとうございました。

補足

確かに三項演算子を利用すると、スマートなコードになりますね。std::string を頭に入れておきたいと思います。

回答No.1

Tacosan

for の条件にある
i<(signed)strlen(s)
を評価するごとに strlen を呼び出すよね.

お礼

ご回答、どうもありがとうございました。

補足

int strch_idx(const char* s, char c){
int temp;
int len = (signed)strlen(s);

for(int i=0; i<len; i++){
～...
とすることで、解決しました。

ご指導、どうもありがとうございました。