HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

Simple Word Search Game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
wordgamesimplesearch

Problem

I've created a fairly simple word search generator/solver. I'm looking to improve on picking the right algorithm to tackle problems like this, so any criticisms on my code would be greatly appreciated :)

```
#include
#include
#include
#include
#include //for stop watch
#include //for failure sound

struct point{ //represents a point in the grid
int x;
int y;
};

char grid[100][100]; //the answer grid
int color[100][100];
int ctr=0;

int grid_size; //size of the grid is grid_size*grid_size

char nullchar='z'; //the NULL character chosen for the grid because the grid will never have lowercase alphabets so any lowercase alphabet can be used as NULL character

int max_words; //number of words that are inserted in the grid

enum direction{ //enumerator for direction in the grid in which the word is inserted
UP=1, //start the count from 1
DOWN,
LEFT,
RIGHT,
UP_LEFT,
UP_RIGHT,
DOWN_LEFT,
DOWN_RIGHT
};

/data to choose the words from/
//
char *animals[]={"deer","sheep","dog","cat","lion","tiger","elephant","wolf","hen"};
int animals_size=9;
char *fruits[]={"banana","apple","pear","grapes","guava","papaya","tomato","mango","lychee","pineapple","cherry"};
int fruits_size=11;
char *games[]={"cricket","baseball","football","tennis","rugby","boxing","wrestling","squash","bowling"};
int games_size=9;
char *starwars[]={"boba","anakin","yoda","maul","palpatine","vader","lightsaber","hansolo","millenium","llando"};
int starwars_size=10;

char *input[100]; //the words that are inserted in the grid
int input_point[100][3];//the x,y coordinate and direction of the words that are inserted in the grid
int mark[100]={0};//used to mark which all words have been identified by the user so that there is no duplicate answer accepted

int flag=0; //flag

struct point shift_point(struct point start, enum direction d){ //start is the position of the point to be shifted in the grid, d is the direction in which the point has to shift
in

Solution

Adding to what other reviews have already mentioned:

-
Try to avoid global data. However, if you do use them, the least harmful kind of globals would be the file scoped ones. How to make a variable or function file scoped in C? Prefix the variable type with the static qualifier when declaring it. E.g.:

static char grid[100][100]; 
static int color[100][100];
static int ctr = 0;


Now those variables cannot be hijacked by other source files in your project by means of extern. This is good since now the programmer has a hint that those variables are not changed outside the current source file. This can save someone from a lot of stress when trying to debug complex code in multi source file projects.

-
Reiterating on the constant point: Mark as much things as you can with const. The great majority of software bugs stem from the programmer not knowing in which state his/her program is. By making variables and pointers const, you have a compiler enforced guarantee that the data won't be changed inside a given context, thus anyone should be able to track down its current state by simple backtracking til the declaration. Apply const to your immutable string arrays, like animals and fruits, but also to function parameters that take read-only pointers to external data, like in:

int check_insert(const char* word, struct point start, enum direction d)
                 ^^^^^


word is not supposed to be changed by the function. So make that a hard constraint.

-
You could typedef the point structure and the direction enum to make usage more succinct, without having to add the struct/enum tags all the time:

typedef struct {
     int x;
     int y;
} point;


Now you can reference it simply as point. Same could be done to direction.

-
You should pick a more consistent naming style and stick to it. Right now, your code has a mix of snake_case and camelCase, which make it harder to read and remember. Pick one notation and use it consistently through the code. You might also consider using something like PascalCase to differentiate User Defined Types from native types and function. E.g. Point and Direction instead.

-
In check_insert(), you should avoid calling strlen() inside the condition check of the while loop:

while(i < (int)strlen(word))


Hoisting that call is an easy optimization for the compiler to do, but still, it looks silly in the code. You should instead store that value inside a local variable. While you are at it, be consistent with the types and use size_t for everything, which is the type returned by strlen(), removing the type cast:

size_t i = 0;
size_t wordLen = strlen(word);
while (i < wordLen) {
...


-
Avoid noisy and obvious comment, like "loop variable" and "function to generate a random char" by the side of a generate_random_char(). This is pure visual pollution. Use comments to explain why you did something. Comments explaining what the code does are an indicative of poorly written code, since the reader should be able to figure out what's being done just by reading the code itself, if it is clear enough.

-
I suggest better spacing your code. A space between arithmetical operators and operands is generally good and facilitates reading, for the same reasons that we use spaces when writing plain text. For example:

for(i=0;i<grid_size;i++){


Versus:

for (i = 0; i < grid_size; i++) {


-
If your code is targeting C99 and above (which you should definitely for new code; older standards are only for maintaining legacy projects), then you can and should declare variables as close to their first use as it is possible, with the intent of reducing scope. Variables declared at the top of a function are like mini-globals. C99 also allows you to declare for loop counters directly inside the loop. E.g.:

for (int i = 0; i < grid_size; i++) {
    for (int j = 0; j < grid_size; j++) {


-
A few variables in your code have very vague names, such as flag, arr and mark. Try to be more descriptive of their purpose.

Code Snippets

static char grid[100][100]; 
static int color[100][100];
static int ctr = 0;
int check_insert(const char* word, struct point start, enum direction d)
                 ^^^^^
typedef struct {
     int x;
     int y;
} point;
while(i < (int)strlen(word))
size_t i = 0;
size_t wordLen = strlen(word);
while (i < wordLen) {
...

Context

StackExchange Code Review Q#88733, answer score: 10

Revisions (0)

No revisions yet.