Brug af struct

Personligt tror jeg, du har taget det rigtige valg ved at bruge en struct med en char* og int i stedet for to arrays af char* og int. Pair er en konceptuel dataenhed i din app, så det giver mening at sætte dem sammen. Hvis du havde 2 separate arrays, ville det være let for dem at komme ud af synkronisering og svært at debugge, hvorfor det skete. Dejligt arbejde!

Foretrækker const og fungerer frem for makroer

Du har defineret arraystørrelsen ved hjælp af en makro. Dette sætter dig dårligt under programmeringen. Du har fjernet typeoplysningerne for værdien. Ved at gøre det:

const int ARR_SIZE = 10; 

får du typesikkerhed. Rediger: At “sa C ++ – ism, der ikke gør” t arbejde i C. Min dårlige! Men resten af rådene i næste afsnit er korrekte, så vidt jeg ved.

Med makroer, der tager parametre, risikerer du, at de bruges på uventede måder og forårsager problemer med fejlfinding. Heldigvis har du ikke gjort det her. Men generelt, hvis du finder ud af at nå ud til en makro, så spørg dig selv, om du ville have det bedre med en konstant eller en funktion. Du vil næsten altid være (forudsat at du kan bruge konstanten på den ønskede måde).

Fejl

Der er nogle fejl i din kode. I make_pair() skal du ikke tjekke for at se hvis malloc() lykkedes. Hvis det fejler, tildeles ingen hukommelse, og pair peger på NULL. Når du prøver at tildele pair->val eller pair->word, vil du gå ned.

Hvis du fik ordnet det, table_insert() bruger resultatet af make_pair() uden at kontrollere, om det “s NULL først. Dette vandt” t går ned med det samme, fordi du bare tildeler matrixmarkøren i tbl->pairs[tbl->sz] for at have værdien pair. Hvad der sker er senere, når du forsøger at søge eller udskrive eller indsætte et andet emne, du får et nedbrud, når du gentager denne post i tabellen og prøver at gøre noget med det.

Du kan muligvis gøre disse fejl umulige ved ikke at dynamisk tildeling af matrixindgange. Lav simpelthen arrayet til et array med Pair struct snarere end en markør til dem.

Navngivning

Mange af dine navne er rigtig gode . Pair og Table er anstændige, læsbare navne til denne opgave. make_pair(), table_insert() osv. er informative. Nogle kunne dog forbedres. tbl sparer dig ikke meget for at skrive table. Brug bare hele ordet. Samme med sz vs. size. i er acceptabelt som et loop-variabelnavn, men det ville være endnu bedre, hvis det var mere beskrivende. For eksempel entry_index eller pair_index. Den skal beskrive, hvad du gentager.

Kommentarer

• når jeg ændrer definitionen til const int ARR_SIZE = 10; det giver mig en fejl i struct Table { struct Pair* pairs[ARR_SIZE]; size_t sz; }; at ARR_SIZE ikke er const, så hvordan skal jeg bruge det i dette tilfælde?
• Undskyld – at ‘ sa C ++ – ism. Jeg troede, det var et C ++ spørgsmål. Min dårlige. Jeg ‘ opdaterer mit svar.
• Jeg ‘ er en stærk tilhænger af beskrivende variabelnavne, men med enkelheden af sløjferne i dette program synes jeg variabelnavnet i fungerer fint.
• Hvis malloc() mislykkes, tildeles pair->val eller pair->word kan gå ned, hvis du er heldig . Det kan bare fortsætte og give forkerte resultater, eller det kan gøre noget helt uventet. At ‘ er glæden ved udefineret adfærd!

• Kast ikke resultatet af malloc.

  malloc returnerer void *, og i C er det gyldigt at konvertere void * til enhver markør. Hvis du kaster for at undertrykke en advarsel om heltal til pointerkonvertering, betyder det, at en compiler ikke har en malloc prototype, og som standard er dens returnering af en int (det er en gammel C-konvention). Hvis int og markøren nu er af forskellig størrelse, ville malloc-returværdien blive afkortet med alle grimme konsekvenser.

• Det anbefales ikke at sizeof(type). Foretrækker sizeof(expression). Overvej i dit tilfælde

   struct Pair * pair = malloc(sizeof(*pair)); 

• table_insert indsættes blindt i en fuld tabel. Den skal teste for tbl->sz < ARR_SIZE, og returner en fejlindikation, hvis den ikke er det.

• Den aktuelle indsættelse

   struct Pair* pair = make_pair(word, val); tbl->pairs[tbl->sz] = pair; // add pair at the last position 

  skal egentlig være en enkelt linje:

   tbl->pairs[tbl->sz] = make_pair(word, val); 

Kommentarer

• hvorfor er det ikke en god idé at caste malloc? I c ++ ville det endda få en compiler-fejl?
• @Sa ndro4912 Spørgsmålet er mærket C. C ++ er et andet sprog, især i denne henseende. Hvis C-compileren nu klager, betyder det, at malloc prototypen mangler, hvilket kan føre til grimme problemer.
• ja jeg ved er c. Jeg blev bare spekuleret på, hvorfor det her ikke er en god praksis at angive typen ændringer med en rollebesætning
• @ Sandro4912 Se redigering.
• @ Sandro4912 pair = malloc(sizeof *pair) er lettere at kode korrekt første gang, lettere at gennemgå og vedligeholde.

Aktivér alle advarsler om kompilator

Med en velaktiveret kompilator skulle for (int i = 0; i < tbl->sz; ++i) have advaret om at blande tegn på i og tbl->sz samt rækkevidde. Spar tid, og aktiver alle advarsler, og brug for (size_t i = 0; i < tbl->sz; ++i).

Generelt er kode squishy ved at bruge int,size_t næsten ombytteligt . Jeg ville kun re-arkitekt og bruge size_t.

Blandet brug af lav og dyb kopi

make_pair(const char* word, int val) tildeler og danner en helt ny struct Pair (dyb kopi), men kopierer dog ikke, hvad word peger på.

Måske

// pair->word = word; pair->word = strdup(word); 

Brug const

search_word() ændrer ikke *tbl, så brug const to convey that. Same for table_find () , table_print () , print_search_result () `.

// int search_word(struct Table* tbl, const char* word) int search_word(const struct Table* tbl, const char* word) 

Navngivning

Kode bruger const char* word; alligevel er det ikke et “ord”, men en streng som brugt i strcmp().

—– Tilføjelser

Overtrædelse af kontrakt?

Intet i kravene “Implementere en opslagstabel med operationer som …” indikerer at const char* er en markør til en streng Så at kalde strcmp() er tvivlsom, medmindre der ikke er angivet krav. Som en char * kunne kode bruge en simpel sammenligning

// if (strcmp(tbl->pairs[i]->word, word) == 0) if (tbl->pairs[i]->word == word) 

Brug af assert()

er mine anvendelser af påstand passende?

Hvis char * markøren, der skal tilføjes / søges, er ikke angivet til at være en streng , assert(word); er ikke korrekt, da word == NULL vides ikke at være ugyldig.

assert(return_val) i table_find(struct Table* tbl, const char* word, int* return_val) er OK, men alligevel ville jeg redesigne return_val == NULL

if (i != -1) { // add if (return_val) { *return_val = tbl->pairs[i]->val; } return 0; }