patterncMinor
A set of functions that supports a spell checker
Viewed 0 times
spellsupportsthatfunctionscheckerset
Problem
This is a program that loads a dictionary into a hash-table, and spell checks a text file provided as a command-line argument. The dictionary is formatted as such (just alphabetically):
When the program is done I am provided with output that looks like :
This is part of a problem set where I was handed a base to build atop of (speller.c), which I'll provide below but am not looking for input on. Despite the goal being to create the most efficient and fastest code possible, I'd rather have input mostly focused on my code-style in forms such as readability and commenting. Again, if you decide to read through speller.c, please do not focus your critique on as it was not written by me.
Some of the requirements are:
speller.c
```
#include
#include
#include
#include
#include "dictionary.h"
#undef calculate
#undef getrusage
// default dictionary
#define DICTIONARY "dictionaries/large"
// prototype
double calculate(const struct rusage b, const struct rusage a);
int main(int argc, char* argv[])
{
// check for correct number of args
if (argc != 2 && argc != 3)
{
printf("Usage: speller [dictionary] text\n");
return 1;
}
// structs for timing data
struct rusage before, after;
// benchmarks
double time_load = 0.0, time_check = 0.0, time_s
python
bar
foo
code
reviewWhen the program is done I am provided with output that looks like :
This is part of a problem set where I was handed a base to build atop of (speller.c), which I'll provide below but am not looking for input on. Despite the goal being to create the most efficient and fastest code possible, I'd rather have input mostly focused on my code-style in forms such as readability and commenting. Again, if you decide to read through speller.c, please do not focus your critique on as it was not written by me.
Some of the requirements are:
- You may assume that any dictionary passed to your program will be structured exactly like ours, lexicographically sorted from top to bottom with one word per line, each of which ends with \n. You may also assume that dictionary will contain at least one word, that no word will be longer than LENGTH (a constant defined in dictionary.h) characters, that no word will appear more than once, and that each word will contain only lowercase alphabetical characters and possibly apostrophes.
- You may assume that check will only be passed strings with alphabetical characters and/or apostrophes.
- Your implementation of check must be case-insensitive.
speller.c
```
#include
#include
#include
#include
#include "dictionary.h"
#undef calculate
#undef getrusage
// default dictionary
#define DICTIONARY "dictionaries/large"
// prototype
double calculate(const struct rusage b, const struct rusage a);
int main(int argc, char* argv[])
{
// check for correct number of args
if (argc != 2 && argc != 3)
{
printf("Usage: speller [dictionary] text\n");
return 1;
}
// structs for timing data
struct rusage before, after;
// benchmarks
double time_load = 0.0, time_check = 0.0, time_s
Solution
-
Recommend eliminating all global variables except for the few needed and make those
-
Missing
-
Recommend eliminating all global variables except for the few needed and make those
static. Aside from hashtable[], re-architect the rest to make local,-
Missing
#include "dictionary.h", Certainly this should be included in the post.-
dictionary.c should insure dictionary.h is self including all needed ` files. By having dictionary.c include dictionary.h first, that is easily tested.
#include "dictionary.h"
#include
#include
...
//#include "dictionary.h"
-
LENGTH is not defined, Hopefully such a generic named define would not be in "dictionary.h". If it is, suggest a dictionary like name DICTIONARY_BUFFER_LENGTH. Same for HASHTABLE_SIZE.
-
node is not declared. Like above, node is a very generic type name. Suggest dictionary_noode or the like.
-
Use type size_t as the array index/size type. Type int may be insufficient.
// for (int i = 0, wordlen = strlen(word); i < wordlen; i++) {
for (size_t i = 0, wordlen = strlen(word); i < wordlen; i++) {
-
buffer[] null character termination need only occur once after the loop.
// for (int i = 0, wordlen = strlen(word); i < wordlen; i++) {
// buffer[i] = tolower(word[i]);
// buffer[i + 1] = '\0'; // Make sure the word gets terminated
// }
size_t = wordlen = strlen(word);
for (size_t i = 0; i < wordlen; i++) {
buffer[i] = tolower(word[i]);
}
buffer[wordlen] = '\0';
-
word, next not declared in current_head_node->word, current_head_node->next;
-
As the declaration of node* current_head_node; is far from the malloc() call,a review takes extra time to know if the type is right. For code that is easier to write correctly, review and maintain, consider.
// current_head_node = malloc(sizeof(node));
current_head_node = malloc(sizeof *current_head_node);
-
Code lacks overflow protection.
// fscanf(dictionary_ref, "%s", buffer)
fscanf(dictionary_ref, "%(some_value)s", buffer)
-
Test not needed. free(NULL) is well defined, effectively a no-op.
// if (previous_head != NULL) free(previous_head);
free(previous_head);
-
Goal: "Your implementation of check must be case-insensitive." If limited to A-Za-z`, converting to lower or upper makes little difference. When using extended (8-bit) character sets, robust code would use a round-trip as extended alphabets do not always have a 1-to-1 mapping.ch = toupper(tolower((unsigned char)ch));Code Snippets
#include "dictionary.h"
#include <string.h>
#include <ctype.h>
...
//#include "dictionary.h"// for (int i = 0, wordlen = strlen(word); i < wordlen; i++) {
for (size_t i = 0, wordlen = strlen(word); i < wordlen; i++) {// for (int i = 0, wordlen = strlen(word); i < wordlen; i++) {
// buffer[i] = tolower(word[i]);
// buffer[i + 1] = '\0'; // Make sure the word gets terminated
// }
size_t = wordlen = strlen(word);
for (size_t i = 0; i < wordlen; i++) {
buffer[i] = tolower(word[i]);
}
buffer[wordlen] = '\0';// current_head_node = malloc(sizeof(node));
current_head_node = malloc(sizeof *current_head_node);// fscanf(dictionary_ref, "%s", buffer)
fscanf(dictionary_ref, "%(some_value)s", buffer)Context
StackExchange Code Review Q#141711, answer score: 2
Revisions (0)
No revisions yet.