これは、厳密なANSIC89ペダンティックコードであると想定されています。 [word1] word2 [[word1] word2 [ word3]そして他の形式で失敗を返します。
動作しているように見えますが、とても醜いようです。 GetTokenBetweenSquareBraces
とGetTokenBtweenOpositeSquareBraces
が重複しているという事実についてコメントする必要はありません。
方法に関するヒントをいくつか教えてください。これをクリーンアップします。
#include <stdio.h> #include <string.h> #include <ctype.h> char * TrimWhiteSpaces(char *str) { char *out = str; int i; int len = strlen(str); for (i=0; i<len && isspace(str[i]); i++, out++); /*scan forward*/ for (i=len-1; i>=0 && isspace(str[i]); str[i]=0, i--);/*scan backward*/ return out; } char * GetTokenBetweenSquareBraces(char * input, char **output, int * output_size) { char *p = TrimWhiteSpaces(input); *output_size=0; if (p[0] == "[") { *output = TrimWhiteSpaces(p + 1); do { (*output_size)++; }while((*output)[*output_size] != "]" && isalnum((*output)[*output_size])); } else { return NULL; } return (*output) + *output_size; } char * GetTokenBtweenOpositeSquareBraces(char * input, char **output, int * output_size) { char *p = TrimWhiteSpaces(input); *output_size=0; if (p[0] == "]") { *output = TrimWhiteSpaces(p + 1); do { (*output_size)++; }while((*output)[*output_size] != "[" && isalnum((*output)[*output_size])); } else { return NULL; } return (*output) + *output_size; } int GetWords(char * str,char * word1,char * word2,char * word3) { char * next=NULL,*output=NULL; int outputsize; printf ("\nSplitting string \"%s\" into tokens:\n",str); next = GetTokenBetweenSquareBraces (str,&output,&outputsize); strncpy(word1,output,outputsize); word1[outputsize] = "\0"; strcpy(word1,TrimWhiteSpaces(word1)); if(!next) return 0; next = GetTokenBtweenOpositeSquareBraces (next,&output,&outputsize); strncpy(word2,output,outputsize); word2[outputsize] = "\0"; strcpy(word2,TrimWhiteSpaces(word2)); if(!next) return 0; next = GetTokenBetweenSquareBraces (next,&output,&outputsize); strncpy(word3,output,outputsize); word3[outputsize] = "\0"; strcpy(word3,TrimWhiteSpaces(word3)); if(!next) return 0; return 1; } void TestGetWords(char * str ) { char word1[20],word2[20],word3[20]; if (GetWords(str,word1,word2,word3)) { printf("|%s|%s|%s|\n",word1,word2,word3); } else { printf("3ViLLLL\n"); } } int main (void) { char str[] ="[ hello ] gfd [ hello2 ] "; char str2[] ="[ hello [ gfd [ hello2 ] "; char str3[] ="the wie321vg42g42g!@#"; char str4[] ="][123[]23][231["; TestGetWords(str); TestGetWords(str2); TestGetWords(str3); TestGetWords(str4); getchar(); return 1; }
コメント
- まず、インデントを修正します。マークダウンエンジンは’タブとは異なります。タブをスペースに置き換えてください。
- @LokiAstari:元のコードを置き換えたようです。
- Opps。ごめんなさい。私の失敗を修正したと思います。メタに関する記事からコードを入手しました。タブの問題を修正して元に戻しました。これが正しくない場合は申し訳ありませんが、’バージョンをロールバックできないようです。
- @LokiAstari:はい、見た目はずっと良くなっています。
回答
#include <stdio.h> #include <string.h> #include <ctype.h> char * TrimWhiteSpaces(char *str) { char *out = str; int i; int len = strlen(str); for (i=0; i<len && isspace(str[i]); i++, out++); /*scan forward*/
少なくともここにコメントがあります。セミコロンを見逃しがちです。i < len
テストは必要ないと思います。文字列の末尾の0は、isspace
テストに失敗するはずなので、長さも確認する必要はありません。また、追跡することもあまり意味がありません。 i
の。代わりに、out
を使用してください。
for (i=len-1; i>=0 && isspace(str[i]); str[i]=0, i--);/*scan backward*/
これらすべてのスペースを0に設定する必要はありません。全体として、その1行で多くの作業を行っています。ループ制御とは関係がないため、少なくともループ本体内で0設定のみを行う必要があります。
return out;
通常、パラメータを変更するか、新しいパラメータを返すのが最善です。両方を行わないでください。ここでは、新しい文字列ポインタを返し、元の文字列を変更します。
} char * GetTokenBetweenSquareBraces(char * input, char **output, int * output_size) { char *p = TrimWhiteSpaces(input); *output_size=0; if (p[0] == "[") { *output = TrimWhiteSpaces(p + 1); do { (*output_size)++; }while((*output)[*output_size] != "]" && isalnum((*output)[*output_size]));
]
は数字でも文字でもありません。これらのテストの両方は必要ありません。
} else { return NULL; } return (*output) + *output_size; } char * GetTokenBtweenOpositeSquareBraces(char * input, char **output, int * output_size) { char *p = TrimWhiteSpaces(input); *output_size=0; if (p[0] == "]") { *output = TrimWhiteSpaces(p + 1); do { (*output_size)++; }while((*output)[*output_size] != "[" && isalnum((*output)[*output_size])); } else { return NULL; } return (*output) + *output_size; }
デジャヴ!これは前の関数とほとんど同じです。ブラケットの方向のみが逆になっています。そのコードを共有できるはずです。
int GetWords(char * str,char * word1,char * word2,char * word3) { char * next=NULL,*output=NULL; int outputsize; printf ("\nSplitting string \"%s\" into tokens:\n",str);
一般的に、作業関数に出力を行わせないことをお勧めします。また、改行を配置する場所の奇妙な選択。
next = GetTokenBetweenSquareBraces (str,&output,&outputsize); strncpy(word1,output,outputsize); word1[outputsize] = "\0"; strcpy(word1,TrimWhiteSpaces(word1));
なぜここで空白をトリミングするのですか?すでにそれをしていませんでした。テキストをコピーするために多くの作業を行っています。おそらく、GetTokenBetweenSquareBracesが行うべきことでしたか?
if(!next) return 0; next = GetTokenBtweenOpositeSquareBraces (next,&output,&outputsize); strncpy(word2,output,outputsize); word2[outputsize] = "\0"; strcpy(word2,TrimWhiteSpaces(word2)); if(!next) return 0;
既視感!
next = GetTokenBetweenSquareBraces (next,&output,&outputsize); strncpy(word3,output,outputsize); word3[outputsize] = "\0"; strcpy(word3,TrimWhiteSpaces(word3)); if(!next) return 0;
デジャヴ!
return 1; } void TestGetWords(char * str ) { char word1[20],word2[20],word3[20];
コードは、これらの変数をオーバーフローさせないように注意していません。それについて何かしたいと思うかもしれません
if (GetWords(str,word1,word2,word3)) { printf("|%s|%s|%s|\n",word1,word2,word3); } else { printf("3ViLLLL\n"); } } int main (void) { char str[] ="[ hello ] gfd [ hello2 ] "; char str2[] ="[ hello [ gfd [ hello2 ] "; char str3[] ="the wie321vg42g42g!@#"; char str4[] ="][123[]23][231["; TestGetWords(str); TestGetWords(str2); TestGetWords(str3); TestGetWords(str4);
自動テストの目的のために、正しい答えを提供し、コードでそれと照合する方が実際には良いでしょう。そうすれば、プログラムはいつ間違っているかを教えてくれます。
getchar(); return 1;
0は、プログラムの実行が成功したことを示すために使用されます。
}
全体的に、間違った語彙を使用しているため、プログラムは醜いです。あなたは「タスクを説明しやすくする語彙を定義する代わりに、与えられた語彙を採用しました。これがあなたの問題に対する私のアプローチです
char * Whitespace(char * str) /* This function return the `str` pointer incremented past any whitespace. */ { /* when an error occurs, we return NULL. If an error has already occurred, just pass it on */ if(!str) return str; while(isspace(*str)) { str++; } return str; } char * Character(char * str, char c) /* This function tries to match a specific character. It returns `str` incremented past the character or NULL if the character was not found */ { if(!str) return str; /* Eat any whitespace before the character */ str = Whitespace(str); if(c != *str) { return NULL; } else { return str + 1; } } char * Word(char * str, char * word) /* This function reads a sequence of numbers and letter into word and then returns a pointer to the position after the word */ { /* Handle errors and whitespace */ if(!str) return str; str = Whitespace(str); /* copy characters */ while(isalnum(*str)) { *word++ = *str++; } *word = 0; /* don"t forget null!*/ return str; } int GetWords(char * str,char * word1,char * word2,char * word3) { str = Character(str, "["); str = Word(str, word1); str = Character(str, "]"); str = Word(str, word2); str = Character(str, "["); str = Word(str, word3); str = Character(str, "]"); str = Character(str, "\0"); return str != NULL; }
私は何ですか」実行した(または実行しようとした)のは、Character、Whitespace、およびWordの関数を非常に単純になるように記述することです。 char *
を理解していれば、問題はないはずです。しかし、これらのシンプルなツールは非常にうまく組み合わされて、パーサーを簡単に実装できます。
コメント
- +1 for “通常、作業関数に出力を行わせないことをお勧めします”。また、非常に優れたクリーンなソリューションです。
回答
これはおそらく少し醜いですが、ただし、Cでは文字列の処理がきれいになることはありません。
static const char * skip_space(const char *s) { return s + strspn(s, " "); } static const char * skip_bracket(const char * s, int bracket) { s = skip_space(s); if (*s != bracket) return NULL; return skip_space(++s); } static const char * skip_word(const char * s) { return s + strcspn(s, " []"); } static const char * copy_word(char *w, const char *s, size_t size) { const char * end = skip_word(s); size_t len = end - s; if (len >= size) /* silently truncate word to buffer size */ len = size - 1; memcpy(w, s, len); w[len] = "\0"; return skip_space(end); } static int get_words(const char *s, char *w1, char *w2, char *w3, size_t size) { if ((s = skip_bracket(s, "[")) == NULL) return 0; s = copy_word(w1, s, size); if ((s = skip_bracket(s, "]")) == NULL) return 0; s = copy_word(w2, s, size); if ((s = skip_bracket(s, "[")) == NULL) return 0; s = copy_word(w3, s, size); if ((s = skip_bracket(s, "]")) == NULL) return 0; return 1; }
回答
ステートマシンを使用してこのタスクを完了することができます。
#include <stdio.h> #include <string.h> void Tokenize(char* s) { // the following array return new state based on current state and current scanned char // Input: * [ ] space Print Tokenize Current State Expression /*Next state:*/char StateArray[12][3][4] = {{{11,1,11,0} ,{0,0,0,0},{0,0,0,0} }, //0 {space}*{[} {{2,11,11,1} ,{1,0,0,0},{0,0,0,0}}, //1 {space}*{char} {{2,11,4,3} ,{1,0,0,0},{0,0,1,0}}, //2 {char}*{space}?{]} {{11,11,4,3} ,{0,0,0,0},{0,0,1,0}}, //3 {space}*{]} {{5,11,11,4} ,{1,0,0,0},{0,0,0,0}}, //4 {space)*{char} {{5,7,11,6} ,{1,0,0,0},{0,1,0,0}}, //5 {char}*{space}?{[} {{11,7,11,6} ,{0,0,0,0},{0,1,0,0}}, //6 {space}*{[} {{8,11,11,7} ,{1,0,0,0},{0,0,0,0}}, //7 {space}*{char} {{8,11,10,9} ,{1,0,0,0},{0,0,1,0}}, //8 {char}*{space}?{]} {{11,11,10,9} ,{0,0,0,0},{0,0,1,0}}, //9 {space}*{]} {{11,11,11,10} ,{0,0,0,0},{0,0,0,0}}, //10 {space}* {{11,11,11,11} ,{0,0,0,0},{0,0,0,0}} }; char state=0; int len = strlen(s); for(int i =0;i<len;i++) { if(StateArray[state][1][(s[i]^91)? ((s[i]^93)?((s[i]^32)? 0:3):2):1]) printf("%c",s[i]); if(StateArray[state][2][(s[i]^91)? ((s[i]^93)?((s[i]^32)? 0:3):2):1]) printf("\n"); state=StateArray[state][0][(s[i]^91)? ((s[i]^93)?((s[i]^32)? 0:3):2):1]; switch(state) { case 11: printf("Error at column %d",i); case 10: if(i==len-1) { printf("\nParsing completed"); } } } } int main(void) { char* s= " [ word1 ] word2word [ 3 ] "; // test string Tokenize(s); }
コメント
- こんにちは、そしてコードレビューへようこそ。このコードは実際にはレビューではありません。むしろ、それが何をするのか、なぜそれが機能するのか、そしてなぜそれが元のコードよりも優れているのかについてほとんど説明せずに物事を行うための代替方法です。それを介して、中括弧の欠落、フォールスルーのケースステートメント、および文書化されていないあいまいなビット単位の操作について心配します。これが優れている理由、OPとは異なる方法で解決される理由、およびこれらの選択によってコードが向上する理由について、詳細を追加することを検討してください。
- これを手作業で作成しましたか、それとも何らかのツールが関係していますか? 私はこのコンセプトが好きですが、’これをサポートすることを恐れています。 マジックナンバーはたくさんあります。