Utilizzo di struct

Personalmente, penso che tu abbia fatto la scelta giusta utilizzando un struct con un char* e int al posto di 2 array di char* e int, rispettivamente. Pair è ununità concettuale di dati nella tua app, quindi ha senso metterli insieme. Se avessi 2 array separati, sarebbe facile per loro perdere la sincronizzazione e difficile eseguire il debug del motivo. Ottimo lavoro!

Preferisci const e le funzioni alle macro

Hai definito la dimensione dellarray utilizzando una macro. Questo inserisce sei in svantaggio durante la programmazione. Hai rimosso le informazioni sul tipo per il valore. In questo modo:

const int ARR_SIZE = 10; 

ottieni lindipendenza dai tipi. Modifica: Quello “è un C ++ – ismo che non funziona” t lavoro in C. Colpa mia! Ma il resto del consiglio nel prossimo paragrafo è corretto per quanto ne so.

Con le macro che accettano parametri corri il rischio che vengano utilizzate in modi inaspettati e causino problemi difficili da eseguire il debug. Fortunatamente, non lhai fatto qui. Ma in generale, se ti ritrovi a cercare una macro, chiediti se staresti meglio con una costante o una funzione. Lo sarai quasi sempre (supponendo che tu possa usare la costante nel modo desiderato).

Errori

Ci sono alcuni errori nel codice. In make_pair(), non controlli per vedere se malloc() è riuscito. Se fallisce, non viene allocata memoria e pair punta a NULL. Quando provi ad assegnare pair->val o pair->word ti arresti in modo anomalo.

Se lhai risolto, table_insert() utilizza il risultato di make_pair() senza controllare se “è NULL per primo. Questo ha vinto” t si blocca immediatamente, perché “stai semplicemente assegnando il puntatore allarray in tbl->pairs[tbl->sz] per avere il valore di pair. Quello che succederà è dopo provi a cercare, stampare o inserire un altro elemento, si verificherà un arresto anomalo quando ripeti quella voce nella tabella e provi a fare qualcosa con essa.

Potresti rendere questi errori non possibili se non allocando dinamicamente le voci dellarray. Rendi semplicemente larray un array di Pair strutture piuttosto che un puntatore ad esse.

Denominazione

Molti dei tuoi nomi sono davvero buoni . Pair e Table sono nomi accettabili e leggibili per questa attività. make_pair(), table_insert() e così via sono informativi. Alcuni potrebbero essere migliorati, però. tbl non ti fa risparmiare molto digitando su table. Usa la parola intera. Stessa cosa con sz e size. i è accettabile come nome di variabile di ciclo, ma sarebbe ancora meglio se fosse più descrittivo. Ad esempio, entry_index o pair_index. Dovrebbe descrivere ciò su cui stai “ripetendo.

Commenti

• quando cambio la definizione in const int ARR_SIZE = 10; mi dà un errore in struct Table { struct Pair* pairs[ARR_SIZE]; size_t sz; }; che ARR_SIZE non è const, quindi come usarlo in questo caso?
• Scusa, ‘ sa C ++ – ism. Pensavo fosse una domanda C ++. Colpa mia. ‘ aggiornerò la mia risposta.
• Sono ‘ un convinto sostenitore dei nomi descrittivi delle variabili, ma con la semplicità dei cicli in questo programma penso che il nome della variabile i funziona perfettamente.
• Se malloc() fallisce, assegnazione a pair->val o pair->word potrebbe arrestarsi in modo anomalo, se sei fortunato . Potrebbe semplicemente continuare e dare risultati sbagliati, o potrebbe fare qualcosa di totalmente inaspettato. Questa ‘ è la gioia di un comportamento indefinito!

• Non eseguire il cast del risultato di malloc.

  malloc restituisce void * e in C è valido per convertire void * in qualsiasi puntatore. Se esegui il cast per sopprimere un avviso relativo alla conversione da numero intero a puntatore, significa che un compilatore non ha un prototipo malloc e il suo valore predefinito restituisce un int (è unantica convenzione in C). Ora se int e il puntatore sono di dimensioni diverse, il valore restituito da malloc verrebbe troncato, con tutte le brutte conseguenze.

• Non è consigliabile sizeof(type). Preferire sizeof(expression). Nel tuo caso, considera

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

• table_insert inserisce ciecamente in una tabella completa. Dovrebbe testare tbl->sz < ARR_SIZE e restituisce unindicazione di errore se non lo è.

• Leffettivo inserimento

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

  dovrebbe essere davvero una singola riga:

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

Commenti

• perché non è una buona idea eseguire il cast di malloc? In c ++ si otterrebbe anche un errore del compilatore?
• @Sa ndro4912 La domanda è contrassegnata con C. C ++ è un linguaggio diverso, in particolare sotto questo aspetto. Ora, se il compilatore C si lamenta, significa che manca il prototipo malloc, il che può portare a fastidiosi problemi.
• sì, lo so è c. Mi chiedevo solo perché qui non è una buona pratica indicare i cambiamenti di tipo con un cast
• @ Sandro4912 Vedi modifica.
• @ Sandro4912 pair = malloc(sizeof *pair) è più facile da codificare correttamente la prima volta, più facile da rivedere e mantenere.

Abilita tutto avvisi del compilatore

Con un compilatore ben abilitato, for (int i = 0; i < tbl->sz; ++i) avrebbe dovuto avvisare in caso di combinazione di segni di i e tbl->sz così come lintervallo. Risparmia tempo e abilita tutti gli avvisi e utilizza for (size_t i = 0; i < tbl->sz; ++i).

In generale, il codice è instabile quando si utilizza int,size_t quasi in modo intercambiabile . Ho “riorganizzato e utilizzato solo size_t.

Uso misto di copia superficiale e profonda

make_pair(const char* word, int val) alloca e forma un nuovo struct Pair (copia completa), ma non copia ciò a cui punta word.

Forse

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

Usa const

search_word() non modifica *tbl, quindi utilizza 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) 

Denominazione

Codice utilizza const char* word; ma non è una “parola”, ma una stringa come viene utilizzata in strcmp().

—– Aggiunte

Violazione del contratto?

Niente nei requisiti “Implementa una tabella di ricerca con operazioni come …” indica che const char* è un puntatore a una stringa . Quindi chiamare strcmp() è discutibile a meno che non esistano requisiti non dichiarati. Come char *, il codice potrebbe utilizzare un semplice confronto

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

Utilizzo di assert()

i miei usi di assert sono appropriati?

Se il char * puntatore per aggiungere / cercare non è specificato come una stringa , assert(word); non è corretto in quanto word == NULL non è noto per essere non valido.

Il assert(return_val) in table_find(struct Table* tbl, const char* word, int* return_val) va bene, ma vorrei riprogettare per consentire return_val == NULL

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