Met struct

Persoonlijk denk ik dat je de juiste keuze hebt gemaakt door een struct met een char* en int erin in plaats van 2 arrays van char* en int, respectievelijk. De Pair is een conceptuele gegevenseenheid in uw app, dus het is logisch om ze samen te voegen. Als je twee afzonderlijke arrays had, zouden ze gemakkelijk uit de pas lopen en moeilijk te achterhalen waarom dat gebeurde. Goed gedaan!

Geef de voorkeur aan const en functies boven macros

Je hebt de array-grootte gedefinieerd met een macro. Dit plaatst je bent in het nadeel tijdens het programmeren. Je hebt de type-informatie voor de waarde verwijderd. Door het:

const int ARR_SIZE = 10; 

te maken, krijgt u typeveiligheid. Bewerken: Dat “is een C ++ – isme dat niet” t werk in C. Mijn slecht! Maar de rest van het advies in de volgende paragraaf is correct voor zover ik weet.

Met macros die parameters aannemen, loop je het risico dat ze op onverwachte manieren worden gebruikt en moeilijk te debuggen problemen veroorzaken. Gelukkig heb je dat hier niet gedaan. Maar als je merkt dat je naar een macro reikt, vraag jezelf dan af of je beter af zou zijn met een constante of een functie. Dat zul je bijna altijd zijn (ervan uitgaande dat je de constante op de gewenste manier).

Fouten

Er zijn enkele fouten in uw code. In make_pair() controleer je niet om te zien als malloc() is geslaagd. Als het niet lukt, wordt er geen geheugen toegewezen en pair verwijst naar NULL. Als je pair->val of pair->word probeert toe te wijzen, crash je.

Als je dat hebt opgelost, table_insert() gebruikt het resultaat van make_pair() zonder te controleren of het eerst “s NULL is. Dit heeft gewonnen” t crasht onmiddellijk, omdat je “gewoon de array-pointer toewijst in tbl->pairs[tbl->sz] om de waarde pair te hebben. Wat gebeurt er later wanneer je probeert te zoeken of een ander item af te drukken of in te voegen, je zult een crash krijgen als je dat item in de tabel herhaalt en er iets mee probeert te doen.

Je zou deze fouten niet mogelijk kunnen maken door niet het dynamisch toewijzen van de array-items. Maak de array gewoon een array van Pair structs in plaats van een verwijzing ernaar.

Naamgeving

Veel van je namen zijn echt goed . Pair en Table zijn fatsoenlijke, leesbare namen voor deze taak. make_pair(), table_insert(), etc. zijn informatief. Sommige kunnen echter worden verbeterd. tbl bespaart u niet veel typen over table. Gebruik gewoon het hele woord. Hetzelfde met sz vs. size. i is acceptabel als lusvariabelenaam, maar het zou nog beter zijn als het meer beschrijvend was. Bijvoorbeeld entry_index of pair_index. Het zou moeten beschrijven wat je “herhaalt.

Reacties

• wanneer ik de definitie verander in const int ARR_SIZE = 10; het geeft me een fout in struct Table { struct Pair* pairs[ARR_SIZE]; size_t sz; }; dat ARR_SIZE niet const is, dus hoe gebruik je het in dit geval dan?
• Sorry – dat ‘ sa C ++ – ism. Ik dacht dat dit een C ++ – vraag was. Mijn slechte. Ik ‘ zal mijn antwoord updaten.
• Ik ‘ ben een fervent voorstander van beschrijvende variabelenamen, maar met de eenvoud van de loops in dit programma denk ik dat de variabelenaam i werkt prima.
• Als malloc() mislukt, wordt toegewezen aan pair->val of pair->word kan crashen, als je geluk hebt . Het kan gewoon doorgaan en verkeerde resultaten opleveren, of het kan iets totaal onverwachts doen. Dat ‘ is de vreugde van ongedefinieerd gedrag!

• Cast niet het resultaat van malloc.

  malloc geeft void *, en in C is het geldig om void * naar een willekeurige pointer te converteren. Als je cast om een waarschuwing over de conversie van integer naar pointer te onderdrukken, betekent dit dat een compiler “geen malloc prototype heeft en standaard een (het is een oude C-conventie). Als int en pointer een verschillende grootte hebben, wordt de malloc-retourwaarde afgekapt, met alle nare gevolgen van dien.

• Het wordt niet aanbevolen om sizeof(type). Geef de voorkeur aan sizeof(expression). Overweeg in jouw geval

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

• table_insert voegt blindelings in een volledige tabel in. Het zou moeten testen op tbl->sz < ARR_SIZE en retourneer een foutmelding als dat niet het geval is.

• De daadwerkelijke invoeging

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

  zou eigenlijk een enkele regel moeten zijn:

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

Opmerkingen

• waarom is het geen goed idee om malloc te casten? In c ++ zou het zelfs een compilerfout krijgen?
• @Sa ndro4912 De vraag is getagd met C. C ++ is een andere taal, vooral in dit opzicht. Als de C-compiler nu klaagt, betekent dit dat het malloc prototype ontbreekt, wat tot vervelende problemen kan leiden.
• Ja, ik weet het is c. ik vroeg me gewoon af waarom het hier geen goede gewoonte is om de typeveranderingen met een cast aan te geven
• @ Sandro4912 Zie bewerken.
• @ Sandro4912 pair = malloc(sizeof *pair) is de eerste keer gemakkelijker correct te coderen, gemakkelijker te bekijken en te onderhouden.

Alles inschakelen compilerwaarschuwingen

Met een goed ingeschakelde compiler, for (int i = 0; i < tbl->sz; ++i) had moeten waarschuwen voor het mengen van tekens van i en tbl->sz evenals bereik. Bespaar tijd en schakel alle waarschuwingen in en gebruik for (size_t i = 0; i < tbl->sz; ++i).

Over het algemeen is de code squishy bij het gebruik van int,size_t bijna onderling uitwisselbaar . Ik “heb opnieuw ontworpen en size_t alleen gebruikt.

Gemengd gebruik van ondiepe en diepe kopie

make_pair(const char* word, int val) wijst en vormt een geheel nieuwe struct Pair (diepe kopie), maar kopieert niet waarnaar word verwijst.

Misschien

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

Gebruik const

search_word() wijzigt *tbl niet, dus gebruik 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) 

Naamgeving

Code gebruikt const char* word; maar het is geen “woord”, maar een string zoals gebruikt in strcmp().

—– Aanvullingen

Contractschending?

Niets in de vereisten “Implementeer een opzoektabel met bewerkingen zoals …” geeft aan dat const char* is een pointer naar een string . Dus strcmp() is twijfelachtig, tenzij er niet-aangegeven vereisten bestaan. Als char *, zou code een eenvoudige vergelijking kunnen gebruiken

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

Gebruik van assert()

zijn mijn gebruik van assert geschikt?

Als de char * pointer om toe te voegen / te zoeken is niet gespecificeerd als een string , assert(word); is niet juist als word == NULL staat niet bekend als ongeldig.

De assert(return_val) in table_find(struct Table* tbl, const char* word, int* return_val) is OK, maar ik zou het ontwerp opnieuw willen ontwerpen om return_val == NULL

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