Dette formodes at være streng ANSI C89 pedantisk kode. Det skal udtrække word1
, word2
og word3
fra en strengformateret [word1] word2 [ word3] og returneringsfejl i ethvert andet format.
Det ser ud til at fungere, men det virker så grimt. Ingen grund til at kommentere om, at GetTokenBetweenSquareBraces
og GetTokenBtweenOpositeSquareBraces
er dubletter.
Jeg ville elske nogle tip til, hvordan man ryd dette 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; }
Kommentarer
- Først skal du rette din indrykning. Markdown-motor kan ikke lide ‘ som faner – udskift dem med mellemrum.
- @LokiAstari: det ser ud til, at du har udskiftet hans oprindelige kode.
- Opps. Undskyld. Løst min skrue, håber jeg. Jeg fik koden fra artiklen om meta. løst faneproblemet og sæt det tilbage. Hvis dette ikke er korrekt, er jeg ked af det, men jeg kan ‘ ikke synes at tilbageføre en version.
- @LokiAstari: Ja, det ser meget bedre ud.
Svar
#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*/
Jeg ville i det mindste have en krop med en kommentar til det her. Det er let at gå glip af det semikolon. Jeg tror ikke, du har brug for i < len
testen. 0 i slutningen af strengen skulle mislykkes i isspace
-testen, så du behøver ikke også kontrollere længden. Det giver heller ikke mening at holde styr af i
. Brug i stedet bare out
.
for (i=len-1; i>=0 && isspace(str[i]); str[i]=0, i--);/*scan backward*/
Det er virkelig ikke nødvendigt at indstille alle disse mellemrum til 0. Alt i alt gør du for meget arbejde på den ene linje. Du skal i det mindste kun udføre 0-indstillingen inde i sløjfekroppen, fordi det ikke har noget at gøre med sløjfekontrol.
return out;
Generelt er det bedst at enten ændre dine parametre eller returnere nye. Gør ikke begge dele. Her returnerer du en ny strengmarkør og ændrer den originale streng.
} 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]));
]
er ikke et tal eller et bogstav. Du har ikke brug for begge disse tests.
} 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! Dette er næsten nøjagtigt det samme som den foregående funktion. Kun beslagsretningerne er vendt om. Det ser ud til, at du skal være i stand til at dele den kode.
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);
Generelt anbefaler jeg, at dine arbejdsfunktioner ikke udfører noget. Også mærkeligt valg af, hvor nye linjer skal placeres.
next = GetTokenBetweenSquareBraces (str,&output,&outputsize); strncpy(word1,output,outputsize); word1[outputsize] = "\0"; strcpy(word1,TrimWhiteSpaces(word1));
Hvorfor trimmer du det hvide mellemrum her? Har du ikke allerede gjort det. Du gør en masse arbejde med at kopiere teksten. Måske er det noget, som GetTokenBetweenSquareBraces burde have gjort?
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];
Din kode er ikke forsigtig med at sikre, at du ikke overløber disse variabler. Du kan muligvis gøre noget ved det
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);
Med henblik på automatiseret test er det faktisk bedre, hvis du giver det rigtige svar og kontrollerer det i kode. På den måde fortæller programmet dig, hvornår det er forkert.
getchar(); return 1;
0 bruges til at indikere en vellykket programkørsel.
}
Alt i alt er dit program grimt, fordi du bruger det forkerte ordforråd. Du har taget ordforrådet som givet i stedet for at definere det ordforråd, der gjorde opgaven let at beskrive. Her er min tilgang til dit problem
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; }
Hvad jeg ” vi har gjort (eller prøvet at gøre) er at skrive karaktererne, det hvide mellemrum og Word-funktionerne således, at de virkelig er meget enkle. Hvis du forstår char *
, bør du ikke have nogen problemer med dem. Men disse enkle værktøjer kombinerer meget pænt for at muliggøre en enkel implementering af din parser.
Kommentarer
- +1 for ” Generelt anbefaler jeg, at dine arbejdsfunktioner ikke udfører “. Også meget flot og ren løsning.
Svar
Dette er måske lidt mindre grimt, men strenghåndtering bliver aldrig smuk i 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; }
Svar
Du kan bruge en tilstandsmaskine til at fuldføre denne opgave,
#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); }
Kommentarer
- Hej og velkommen til Code Review. Denne kode er ikke rigtig en anmeldelse. Det er snarere en alternativ måde at gøre ting på med lidt forklaring på, hvad den gør, hvorfor den fungerer, og hvorfor den er bedre end originalen. Derudover ser jeg ikke igennem det og bekymre dig om manglende seler, faldsagssætninger og obskure bitvise manipulationer, der ikke er dokumenteret. Overvej venligst at tilføje detaljer om, hvorfor dette er bedre, og hvad det løser anderledes end OP, og hvorfor disse valg giver bedre kode.
- Oprettede du dette manuelt, eller er der noget værktøj involveret? Jeg kan godt lide konceptet, men jeg ‘ ville være bange for at støtte dette. Der er så mange magiske tal.