Verwenden von struct

Ich persönlich denke, Sie haben mit a die richtige Wahl getroffen struct mit einem char* und int anstelle von 2 Arrays von char* bzw. int. Die Pair ist eine konzeptionelle Dateneinheit in Ihrer App. Daher ist es sinnvoll, sie zusammenzustellen. Wenn Sie zwei separate Arrays hätten, wäre es für sie leicht, nicht mehr synchron zu sein und schwer zu debuggen, warum dies passiert ist. Gute Arbeit!

Bevorzugen Sie const und Funktionen gegenüber Makros

Sie haben die Arraygröße mit einem Makro definiert. Dies setzt Sie sind beim Programmieren im Nachteil. Sie haben die Typinformationen für den Wert entfernt. Wenn Sie Folgendes festlegen:

const int ARR_SIZE = 10; 

erhalten Sie Typensicherheit. Bearbeiten: Dieser „sa C ++ – Ismus, der dies nicht tut“ Ich arbeite nicht in C. Mein schlechtes! Der Rest der Ratschläge im nächsten Absatz ist meines Wissens korrekt.

Bei Makros, die Parameter verwenden, besteht die Gefahr, dass sie auf unerwartete Weise verwendet werden und schwer zu debuggende Probleme verursachen. Zum Glück haben Sie das hier nicht getan. Wenn Sie jedoch nach einem Makro greifen, fragen Sie sich im Allgemeinen, ob Sie mit einer Konstante oder einer Funktion besser dran wären. Sie werden es fast immer sein (vorausgesetzt, Sie können die Konstante verwenden auf die gewünschte Weise).

Fehler

Ihr Code enthält einige Fehler. In make_pair() überprüfen Sie nicht, ob dies angezeigt wird wenn malloc() erfolgreich war. Wenn dies fehlschlägt, wird kein Speicher zugewiesen und pair zeigt auf NULL. Wenn Sie versuchen, pair->val oder pair->word zuzuweisen, stürzen Sie ab.

Wenn Sie dies behoben haben, table_insert() verwendet das Ergebnis von make_pair(), ohne zu überprüfen, ob es zuerst NULL ist. t stürzt sofort ab, da Sie „nur den Array-Zeiger in tbl->pairs[tbl->sz] zuweisen, um den Wert pair zu erhalten. Was passiert, ist später, wenn Wenn Sie versuchen, ein anderes Element zu suchen, zu drucken oder einzufügen, kommt es zu einem Absturz, wenn Sie diesen Eintrag in der Tabelle durchlaufen und versuchen, etwas damit zu tun.

Sie könnten diese Fehler nicht möglich machen, indem Sie nicht Dynamisches Zuweisen der Array-Einträge. Machen Sie das Array einfach zu einem Array von Pair -Strukturen und nicht zu einem Zeiger auf diese.

Benennung

Viele Ihrer Namen sind wirklich gut . Pair und Table sind anständige, lesbare Namen für diese Aufgabe. make_pair(), table_insert() usw. sind informativ. Einige könnten jedoch verbessert werden. tbl erspart Ihnen nicht viel Eingabe über table. Verwenden Sie einfach das ganze Wort. Gleiches gilt für sz vs. size. i ist als Name einer Schleifenvariablen akzeptabel, aber es wäre noch besser, wenn es aussagekräftiger wäre. Beispiel: entry_index oder pair_index. Es sollte beschreiben, worüber Sie „iterieren“.

Kommentare

• , wenn ich die Definition in const int ARR_SIZE = 10; Es gibt mir einen Fehler in struct Table { struct Pair* pairs[ARR_SIZE]; size_t sz; };, dass ARR_SIZE nicht const ist. Wie wird es dann in diesem Fall verwendet?
• Entschuldigung – dass ‚ sa C ++ – ism. Ich dachte, dies sei eine C ++ – Frage. Meine schlechte. Ich ‚ werde meine Antwort aktualisieren.
• Ich ‚ bin ein überzeugter Unterstützer beschreibender Variablennamen, aber aufgrund der Einfachheit der Schleifen in diesem Programm denke ich, dass der Variablenname i funktioniert einwandfrei.
• Wenn malloc() fehlschlägt, wird pair->val oder pair->word könnte abstürzen, wenn Sie Glück haben . Es kann einfach weitergehen und falsche Ergebnisse liefern, oder es kann etwas völlig Unerwartetes bewirken. Das ‚ ist die Freude an undefiniertem Verhalten!

• Das Ergebnis von malloc wird nicht umgewandelt.

  malloc gibt void *, und in C ist es gültig, void * in einen beliebigen Zeiger zu konvertieren. Wenn Sie eine Warnung zur Konvertierung von Ganzzahlen in Zeiger unterdrücken, bedeutet dies, dass ein Compiler keinen malloc -Prototyp hat und standardmäßig einen (es ist eine alte C-Konvention). Wenn nun int und der Zeiger unterschiedlich groß sind, wird der Malloc-Rückgabewert abgeschnitten, mit allen bösen Konsequenzen.

• Es wird nicht empfohlen, sizeof(type) zu verwenden. Bevorzugen Sie sizeof(expression). In Ihrem Fall sollten Sie

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

• table_insert fügt blind in eine vollständige Tabelle ein. Es sollte auf tbl->sz < ARR_SIZE und geben eine Fehleranzeige zurück, wenn dies nicht der Fall ist.

• Die tatsächliche Einfügung

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

  sollte eigentlich eine einzelne Zeile sein:

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

Kommentare

• Warum ist es keine gute Idee, Malloc zu besetzen? In C ++ würde es sogar einen Compilerfehler geben?
• @Sa ndro4912 Die Frage ist mit C gekennzeichnet. C ++ ist eine andere Sprache, insbesondere in dieser Hinsicht. Wenn sich der C-Compiler beschwert, bedeutet dies, dass der Prototyp malloc fehlt, was zu bösen Problemen führen kann.
• Ja, ich weiß, ist c. Ich habe mich nur gefragt, warum es hier nicht empfehlenswert ist, die Typänderungen mit einer Besetzung anzugeben.
• @ Sandro4912 Siehe Bearbeiten.
• @ Sandro4912 pair = malloc(sizeof *pair) ist beim ersten Mal einfacher korrekt zu codieren, einfacher zu überprüfen und zu warten.

Alle aktivieren Compiler-Warnungen

Bei einem gut aktivierten Compiler sollte for (int i = 0; i < tbl->sz; ++i) vor dem Mischen der Vorzeichen von i und tbl->sz sowie Bereich. Sparen Sie Zeit und aktivieren Sie alle Warnungen und verwenden Sie for (size_t i = 0; i < tbl->sz; ++i).

Im Allgemeinen ist der Code bei der Verwendung von int,size_t nahezu austauschbar . Ich würde nur size_t neu entwerfen und verwenden.

Gemischte Verwendung von flacher und tiefer Kopie

make_pair(const char* word, int val) weist eine ganz neue struct Pair (tiefe Kopie) zu und bildet sie, kopiert jedoch nicht, worauf word zeigt.

Vielleicht

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

Verwenden Sie const

search_word() ändert *tbl nicht. Verwenden Sie daher 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) 

Benennung

Code verwendet const char* word;, ist jedoch kein „Wort“, sondern eine Zeichenfolge , wie sie in strcmp() verwendet wird.

—– Ergänzungen

Vertragsverletzung?

Nichts in den Anforderungen „Implementierung einer Nachschlagetabelle mit Operationen wie …“ gibt an, dass const char* ist ein Zeiger auf eine Zeichenfolge . Rufen Sie also strcmp() ist fraglich, sofern keine nicht angegebenen Anforderungen vorliegen. Als char * könnte Code einen einfachen Vergleich verwenden.

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

Verwendung von assert()

Sind meine Verwendungszwecke angemessen?

Wenn Der Zeiger char * zum Hinzufügen / Suchen ist nicht als Zeichenfolge angegeben. assert(word); ist nicht als word == NULL ist nicht als ungültig bekannt.

Die assert(return_val) in table_find(struct Table* tbl, const char* word, int* return_val) ist in Ordnung, aber ich würde neu entwerfen, um return_val == NULL

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