C言語の勉強中です

[eetrt]

コード:

#if 1
//問題⑫
#include <stdio.h>
#include <string.h>
void cString(char *str1, char *str2);
int main(void)
{
	char str1[] = "asaa";
	char str2[] = "aaaa";
	cString(str1, str2);
}
void cString(char *str1 ,char *str2){
	int n_1 = 0;
	int n_2 = 0;
	char* def_1 = &str1[0];
	char* def_2 = &str2[0];
	while (*str1 != '\0'){
		*str1++;
		n_1++;
	}
	while (*str2 != '\0'){
		*str2++;
		n_2++;
	}
	if (n_1 == n_2){
		str1 = def_1;
		str2 = def_2;
		int agreement = 0;
		for (int i = 0; i < n_1; i++){
			if (*str1 == *str2){
				agreement++;
			}
			*str1++;
		}
		if (agreement == n_1){
			printf("一致しました");
		}
		else{
			printf("一致しませんでした");
		}
		
	}
	else if (n_1 < n_2){
		printf("引数１『");
		str1 = def_1;
		for (int i = 0; i < n_1; i++){
			printf("%c", *str1);
			*str1++;
		}
		printf("』の数が少ない\n");
	}
	else if (n_1 > n_2){

		printf("引数２『");
		str2 = def_2;
		for (int i = 0; i < n_2; i++){
			printf("%c", *str2);
			*str2++;
		}
		printf("』の数が少ない\n");
	}
}


#endif

cStringという文字列を比較する関数を作ったのですが、何か、非効率な気がします。
どこを改善したらいいでしょうか？
どなたか教えてください。

[みけCAT]

何も制限が無ければ素直にstrlenやstrcmpを使用するべきだと思いますが、
printf以外の標準ライブラリ関数を使ってはいけない縛りでしょうか？

[みけCAT]

str1,str2,def_1,def_2の型はconst char*にした方がいいと思います。
その方がこの関数を使える対象が多くなります。

[みけCAT]

「*str1++;」という文は、「*(str1++);」という意味であり、最初に*を付けてポインタが指す値を取得する意味がありません。
実行効率には影響しないかもしれないですが、ソースコードが若干長く、また若干わかりずらくなっていると思います。

[かずま]

2つの文字列を同時に先頭から見ていくと、
短いほうの文字列の終わりで結果が出ます。
最初に 2つの文字列の長さを知る必要はありません。

コード:

#include <stdio.h>

void cString(const char *str1, const char *str2);

int main(void)
{
    char str1[100], str2[100];

    while (printf(">> "), scanf("%99s%99s", str1, str2) == 2)
        cString(str1, str2);
    return 0;
}

void cString(const char *str1, const char *str2)
{
    const char *p1 = str1, *p2 = str2;
    int c1, c2, diff = 0;

    while (c1 = *p1++, c2 = *p2++, c1 && c2)
        if (c1 != c2) diff = 1;
    if (c2)
        printf("引数1『%s』の数が少ない\n", str1);
    else if (c1)
        printf("引数2『%s』の数が少ない\n", str2);
    else if (diff)
        puts("一致しませんでした");
    else
        puts("一致しました");
}

実行結果

コード:

>> asaa aaaa
一致しませんでした
>> abc abc
一致しました
>> abc abcd
引数1『abc』の数が少ない
>> abc ab
引数2『ab』の数が少ない
>> ^D または ^Z

[box]

コード:

void cString(char *str1, char *str2);

どうせなら、より汎用性を持たせるために
str1 == str2 ならば0を返す
str1 > str2 ならば正の整数を返す
str1 < str2 ならば負の整数を返す
とかいう風にしてみてはどうでしょうか。これは、標準関数のstrcmp()と同じ考え方です。
そうすれば、他の場面でもcString()が使えるようになると思います。

[みけCAT]

とりあえず、軽く書き直してみました。

コード:

#if 1
//問題⑫
#include <stdio.h>
#include <string.h>
void cString(const char *str1,const char *str2);
int main(void)
{
    char str1[] = "asaa";
    char str2[] = "aaaa";
    cString(str1, str2);
}
/* ポインタが指す先の値を変えないならconstを付けておいた方がよい */
void cString(const char *str1,const char *str2){
    int n_1 = 0;
    int n_2 = 0;
    const char* def_1 = str1; /* この場合は&str1[0]はstr1と同じはず */
    const char* def_2 = str2; /* この場合は&str2[0]はstr2と同じはず */
    /* 標準ライブラリ関数を使っていいならn_1=strlen(str1);でいいと思う。コーナーケースが存在？ */
    while (*str1 != '\0'){
        str1++; /* 無駄に*を付ける必要は無い */
        n_1++;
    }
    /* 標準ライブラリ関数を使っていいならn_2=strlen(str2);でいいと思う。コーナーケースが存在？ */
    while (*str2 != '\0'){
        str2++; /* 無駄に*を付ける必要は無い */
        n_2++;
    }
    if (n_1 == n_2){
        str1 = def_1;
        str2 = def_2;
        int agreement = 1; /* どうせ一致するしか見ないなら、フラグで良い */
        for (int i = 0; i < n_1; i++){
            if (*str1 != *str2){
                agreement=0;
                break; /* 枝刈りによる、実行効率の向上 */
            }
            str1++; /* 無駄に*を付ける必要は無い */
            /* 仕様不明 : str2をインクリメントする必要は無いのか？ */
        }
        if (agreement){
            printf("一致しました");
        }
        else{
            printf("一致しませんでした");
        }
        
    }
    else if (n_1 < n_2){
        /* 素直に一気に出力していいと思う。コーナーケースが存在する？ */
        printf("引数１『%s』の数が少ない\n",str1);
    }
    else if (n_1 > n_2){
        /* 素直に一気に出力していいと思う。コーナーケースが存在する？ */
        printf("引数２『%s』の数が少ない\n",str2);
    }
}


#endif

[eetrt]

すみません
コードミスってました
みなさん前のコードで実行結果は正しかったですか？
もしかしたら私のソフトが故障かもしれません

コード:

#if 1
//問題⑫
#include <stdio.h>
#include <string.h>
void cString(char *str1, char *str2);
int main(void)
{
	char str1[] = "as33a";
	char str2[] = "asaaa";
	cString(str1, str2);
}
void cString(char *str1, char *str2){
	int n_1 = 0;
	int n_2 = 0;
	char* def_1 = &str1[0];
	char* def_2 = &str2[0];
	while (*str1 != '\0'){
		*str1++;
		n_1++;
	}
	while (*str2 != '\0'){
		*str2++;
		n_2++;
	}
	if (n_1 == n_2){
		str1 = def_1;
		str2 = def_2;
		int agreement = 1;//ここミス
		for (int i = 0; i < n_1; i++){
			if (*str1 == *str2){
				agreement++;
			}
			*str1++;
		}
		if (agreement == n_1){
			printf("一致しました");
		}
		else{
			printf("一致しませんでした");
		}

	}
	else if (n_1 < n_2){
		printf("引数１『");
		str1 = def_1;
		for (int i = 0; i < n_1; i++){
			printf("%c", *str1);
			*str1++;
		}
		printf("』の数が少ない\n");
	}
	else if (n_1 > n_2){

		printf("引数２『");
		str2 = def_2;
		for (int i = 0; i < n_2; i++){
			printf("%c", *str2);
			*str2++;
		}
		printf("』の数が少ない\n");
	}
}



#endif

皆さん返信有難うございます
>>by みけCAT » 2014年3月02日(日) 17:12

ありがとう御座いますとても参考になります。
細かいところまでありがとうございました。
素直に一気に出力していいと思う。というところの文字が出力できませんでした。
環境とか書いてませんでしたすみません
環境はVC++2013でやっております。

>>by box » 2014年3月02日(日) 17:11

ありがとうございます
最終的にそうしたいと思います。

>>by かずま » 2014年3月02日(日) 17:09
なるほど
最初に文字数を知らなくておｋですね
とても参考になります。

>>by みけCAT » 2014年3月02日(日) 17:03
ありがとうございます。
そう書いてみますね。

>>by みけCAT » 2014年3月02日(日) 16:59
ありがとうございます
const char*はまだ勉強していないので勉強してきますね。

>>by みけCAT » 2014年3月02日(日) 16:55
勉強でそういうのを自分で作ってみようとしています！

[みけCAT]

みなさん前のコードで実行結果は正しかったですか？

関数の仕様がわからないので、わかりません。
テストケースは公開されていますか？
No: 7の38行目で、str2をインクリメントする必要は無いのですか？

[みけCAT]

素直に一気に出力していいと思う。というところの文字が出力できませんでした。

ごめんなさい、私のミスです。
No:7のコードの48行目～55行目を、以下のコードに書き換えてください。

コード:

    else if (n_1 < n_2){
        /* 素直に一気に出力していいと思う。コーナーケースが存在する？ */
        printf("引数１『%s』の数が少ない\n",def_1);
    }
    else if (n_1 > n_2){
        /* 素直に一気に出力していいと思う。コーナーケースが存在する？ */
        printf("引数２『%s』の数が少ない\n",def_2);
    }

[eetrt]

すみません
改めて見返したら
/* 仕様不明 : str2をインクリメントする必要は無いのか？ */
上str2インクリメントしないとそもそも比較できませんねｗ

コード:

//問題⑫
#include <stdio.h>
#include <string.h>
void cString(char *str1, char *str2);
int main(void)
{
	char str1[] = "as33a";
	char str2[] = "as33a";
	cString(str1, str2);
}
void cString(char *str1, char *str2){
	int n_1 = 0;
	int n_2 = 0;
	char* def_1 = &str1[0];
	char* def_2 = &str2[0];
	while (*str1 != '\0'){
		*str1++;
		n_1++;
	}
	while (*str2 != '\0'){
		*str2++;
		n_2++;
	}
	if (n_1 == n_2){
		str1 = def_1;
		str2 = def_2;
		int agreement = 0;
		for (int i = 0; i < n_1; i++){
			if (*str1 == *str2){
				agreement++;
			}
			*str1++;
			*str2++;
		}
		if (agreement == n_1){
			printf("一致しました");
		}
		else{
			printf("一致しませんでした");
		}

	}
	else if (n_1 < n_2){
		printf("引数１『");
		str1 = def_1;
		for (int i = 0; i < n_1; i++){
			printf("%c", *str1);
			*str1++;
		}
		printf("』の数が少ない\n");
	}
	else if (n_1 > n_2){

		printf("引数２『");
		str2 = def_2;
		for (int i = 0; i < n_2; i++){
			printf("%c", *str2);
			*str2++;
		}
		printf("』の数が少ない\n");
	}
}

年3月02日(日) 17:32

ありがとうございます
出力できました！
素人に付き合ってくれてありがとうございます。
とても勉強になって助かりますｍ－－ｍ