Dit wordt verondersteld strikte ANSI C89 pedante code te zijn. Het moet word1
, word2
en word3
extraheren uit een string met de opmaak [woord1] woord2 [ word3] en retourneerfout in een ander formaat.
Het lijkt te werken, maar het lijkt zo lelijk. Het is niet nodig om commentaar te geven op het feit dat GetTokenBetweenSquareBraces
en GetTokenBtweenOpositeSquareBraces
duplicaten zijn.
Ik zou graag wat tips willen hebben over hoe ruim dit op.
#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; }
Reacties
- Herstel eerst je inspringing. Markdown-engine ‘ houdt niet van tabs – vervang ze door spaties.
- @LokiAstari: het lijkt erop dat je zijn originele code hebt vervangen.
- Opps. Sorry. Ik hoop dat mijn fout is opgelost. Ik heb de code uit het artikel over meta. het tabbladprobleem opgelost en teruggeplaatst. Als dit niet correct is, spijt het me, maar ik kan ‘ een versie niet terugdraaien.
- @LokiAstari: Ja, ziet er een stuk beter uit.
Antwoord
#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*/
Ik “had tenminste een lichaam met een opmerking erin. Het is gemakkelijk om die puntkomma te missen. Ik denk niet dat je de i < len
-test nodig hebt. De 0 aan het einde van de string zou de isspace
-test niet moeten doorstaan en dus hoef je ook niet de lengte te controleren. Het heeft ook geen zin om bij te houden van i
. Gebruik in plaats daarvan gewoon out
.
for (i=len-1; i>=0 && isspace(str[i]); str[i]=0, i--);/*scan backward*/
Het is niet echt nodig om al die spaties op 0 te zetten. Over het algemeen doe je te veel werk op die ene regel. Je zou in ieder geval alleen de 0-instelling binnen de loop-body moeten doen, omdat dit niets te maken heeft met de loop-besturing.
return out;
Over het algemeen is het het beste om uw parameters te wijzigen of nieuwe te retourneren. Doe niet beide. Hier retourneert u een nieuwe stringpointer en wijzigt u de originele string.
} 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]));
]
is geen cijfer of letter. Je hebt deze beide tests niet nodig.
} 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; }
Deja Vu! Dit is bijna precies hetzelfde als de vorige functie. Alleen de beugelrichtingen zijn omgekeerd. Het lijkt erop dat je die code zou moeten kunnen delen.
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);
Over het algemeen raad ik af om je werkende functies enige output te laten doen. Ook vreemde keuze van waar nieuwe regels moeten worden geplaatst.
next = GetTokenBetweenSquareBraces (str,&output,&outputsize); strncpy(word1,output,outputsize); word1[outputsize] = "\0"; strcpy(word1,TrimWhiteSpaces(word1));
Waarom trim je hier witruimte? Heeft u dat nog niet gedaan. U doet veel werk om de tekst te kopiëren. Misschien had GetTokenBetweenSquareBraces dat moeten doen?
if(!next) return 0; next = GetTokenBtweenOpositeSquareBraces (next,&output,&outputsize); strncpy(word2,output,outputsize); word2[outputsize] = "\0"; strcpy(word2,TrimWhiteSpaces(word2)); if(!next) return 0;
Deja Vu!
next = GetTokenBetweenSquareBraces (next,&output,&outputsize); strncpy(word3,output,outputsize); word3[outputsize] = "\0"; strcpy(word3,TrimWhiteSpaces(word3)); if(!next) return 0;
Deja Vu!
return 1; } void TestGetWords(char * str ) { char word1[20],word2[20],word3[20];
Uw code is niet voorzichtig om ervoor te zorgen dat u deze variabelen niet overstroomt. Misschien wilt u daar iets aan doen
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);
Voor geautomatiseerde tests is het eigenlijk beter als u het juiste antwoord geeft en dit in code controleert. Op die manier zal het programma je vertellen wanneer het fout is.
getchar(); return 1;
0 wordt gebruikt om aan te geven dat het programma succesvol is uitgevoerd.
}
Over het algemeen is je programma lelijk omdat je de verkeerde woordenschat gebruikt. Je hebt “het vocabulaire genomen zoals het is gegeven in plaats van het vocabulaire te definiëren dat de taak gemakkelijk te beschrijven maakte. Hier is mijn benadering van je probleem
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; }
Wat ik” heb gedaan (of geprobeerd te doen) is de karakter-, witruimte- en woordfuncties zo schrijven dat ze echt heel eenvoudig zijn. Als je char *
begrijpt, zou je er geen problemen mee moeten hebben. Maar deze eenvoudige tools combineren heel goed om een ongecompliceerde implementatie van je parser mogelijk te maken.
Opmerkingen
- +1 voor ” Over het algemeen raad ik af om je werkende functies enige output te laten doen “. Ook een zeer mooie en schone oplossing.
Antwoord
Dit is misschien iets minder lelijk, maar string-afhandeling zal nooit mooi zijn in 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; }
Answer
Je kunt een toestandsmachine gebruiken om deze taak te voltooien,
#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); }
Reacties
- Hallo, en welkom bij Code Review. Deze code is niet echt een review. Het is eerder een alternatieve manier om dingen te doen met weinig uitleg over wat het doet, waarom het werkt en waarom het beter is dan het origineel. erdoorheen en maak je zorgen over ontbrekende accolades, doorval-case-statements en obscure bitsgewijze manipulaties die niet zijn gedocumenteerd. Overweeg alsjeblieft details toe te voegen waarom dit beter is, en wat het anders oplost dan het OP, en waarom die keuzes leiden tot betere code.
- Heeft u dit met de hand gemaakt of is er een hulpmiddel bij betrokken? Ik hou van het concept, maar ik ‘ zou doodsbang zijn om dit te steunen. Er zijn zoveel magische getallen.