Med struct

Personligen tror jag att du har gjort rätt val genom att använda en struct med en char* och int i stället för två matriser med char* respektive int. Pair är en konceptuell dataenhet i din app, så det är vettigt att sätta ihop dem. Om du hade två separata matriser, skulle det vara lätt för dem att komma ur synk och svårt att felsöka varför det hände. Trevligt arbete!

Föredrar const och fungerar framför makron

Du har definierat arraystorleken med ett makro. Detta sätter du har en nackdel under programmeringen. Du har tagit bort typinformationen för värdet. Genom att göra det:

const int ARR_SIZE = 10; 

får du typsäkerhet. Redigera: Den ”sa C ++ – ism som inte” t fungerar i C. My bad! Men resten av råden i nästa stycke är korrekta så vitt jag vet.

Med makron som tar parametrar riskerar du att de används på oväntade sätt och orsakar problem att felsöka. Lyckligtvis har du inte gjort det här. Men i allmänhet, om du hittar dig själv efter ett makro, fråga dig själv om du skulle ha det bättre med en konstant eller en funktion. Du kommer nästan alltid att vara (förutsatt att du kan använda konstanten på önskat sätt).

Fel

Det finns några fel i din kod. I make_pair() kontrollerar du inte för att se om malloc() lyckades. Om det misslyckas tilldelas inget minne och pair pekar på NULL. När du försöker tilldela pair->val eller pair->word kommer du att krascha.

Om du fixade det, table_insert() använder resultatet av make_pair() utan att kontrollera om det ”s NULL först. Detta vann” t krascha omedelbart, eftersom du bara tilldelar arraypekaren i tbl->pairs[tbl->sz] för att ha värdet pair. Vad som händer är senare när du försöker söka eller skriva ut eller infoga ett annat objekt, du kommer att krascha när du upprepar den posten i tabellen och försöker göra någonting med det.

Du kan göra dessa fel inte möjliga genom att inte dynamiskt allokera matrisposterna. Gör helt enkelt matrisen till en array med Pair -strängar snarare än en pekare till dem.

Namngivning

Många av dina namn är riktigt bra . Pair och Table är anständiga, läsbara namn för den här uppgiften. make_pair(), table_insert(), etc. är informativa. Vissa kan dock förbättras. tbl sparar inte mycket för att skriva över table. Använd bara hela ordet. Samma med sz vs. size. i är acceptabelt som loop-variabelnamn, men det vore ännu bättre om det var mer beskrivande. Till exempel entry_index eller pair_index. Det ska beskriva vad du itererar över.

Kommentarer

• när jag ändrar definitionen till const int ARR_SIZE = 10; det ger mig ett fel i struct Table { struct Pair* pairs[ARR_SIZE]; size_t sz; }; att ARR_SIZE inte är const. Så hur ska jag använda det i det här fallet då?
• Tyvärr – att ’ sa C ++ – ism. Jag tyckte att det här var en C ++ -fråga. Min dåliga. Jag ’ uppdaterar mitt svar.
• Jag ’ är en stark anhängare av beskrivande variabelnamn, men med enkelhetens öglor i detta program tror jag att variabelnamnet i fungerar bra.
• Om malloc() misslyckas, tilldelas pair->val eller pair->word kanske kraschar, om du har tur . Det kan bara fortsätta och ge fel resultat, eller så kan det göra något helt oväntat. Att ’ är glädjen av odefinierat beteende!

• Kasta inte resultatet av malloc.

  malloc returnerar void *, och i C är det giltigt att konvertera void * till valfri pekare. Om du kastar för att undertrycka en varning om heltal till pekarkonvertering betyder det att en kompilator inte har en malloc prototyp, och standardinställningen är att returnera en int (det är en gammal C-konvention). Om int och pekaren är av olika storlek, skulle malloc-returvärdet trunkeras med alla otäcka konsekvenser.

• Det rekommenderas inte att sizeof(type). Föredrar sizeof(expression). Överväg

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

• table_insert infogas blint i en fullständig tabell. Det bör testas för tbl->sz < ARR_SIZE och returnera en felindikering om den inte är det.

• Den faktiska infogningen

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

  borde egentligen vara en enda rad:

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

Kommentarer

• varför är det inte en bra idé att casta malloc? I c ++ skulle det till och med få ett kompileringsfel?
• @Sa ndro4912 Frågan är taggad C. C ++ är ett annat språk, särskilt i detta avseende. Nu om C-kompilatorn klagar betyder det att prototypen malloc saknas, vilket kan leda till otäcka problem.
• ja jag vet är c. Jag undrade bara varför det här inte är en bra praxis att ange typändringarna med en rollbesättning
• @ Sandro4912 Se redigera.
• @ Sandro4912 pair = malloc(sizeof *pair) är lättare att koda korrekt första gången, lättare att granska och underhålla.

Aktivera alla kompilatorvarningar

Med en väl aktiverad kompilator borde for (int i = 0; i < tbl->sz; ++i) ha varnat för att blanda tecken mellan i och tbl->sz samt intervall. Spara tid och aktivera alla varningar och använd for (size_t i = 0; i < tbl->sz; ++i).

I allmänhet är koden snygg när den använder int,size_t nästan omväxlande . Jag ”skulle omarkitekt och endast använda size_t.

Blandad användning av grunt och djupt exemplar

make_pair(const char* word, int val) tilldelar och bildar en helt ny struct Pair (djup kopia), men kopierar ändå inte det word pekar på.

Kanske

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

Använd const

search_word() ändrar inte *tbl, så använd 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) 

Namngivning

Kod använder const char* word; men det är inte ett ”ord” utan en sträng som används i strcmp().

—– Tillägg

Överträdelse av kontrakt?

Ingenting i kraven ”Implementera en uppslagstabell med operationer som …” indikerar att const char* är en pekare till en sträng . Så att ringa strcmp() kan ifrågasättas såvida det inte finns några icke-angivna krav. Som char * kan kod använda en enkel jämförelse

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

Användning av assert()

är min användning av påståenden lämplig?

Om char * pekaren för att lägga till / sökning är inte angiven som en sträng , assert(word); är inte korrekt eftersom word == NULL är inte känt som ogiltigt.

assert(return_val) i table_find(struct Table* tbl, const char* word, int* return_val) är OK, men ändå skulle jag utforma om så att return_val == NULL

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