Snake with ncurses in C

Question

I've implemented a version of snake using GNU ncurses, and I'd appreciate some feedback! There are two main files, a main.c file that houses the main function and a few generic functions, and a snake.c file that houses mostly snake-related functions. I'll include the corresponding header files first, because they help document the implementation.

I used ncurses halfdelay along with a timer in order to manage movement. The levels of difficulty just set different timeouts between movement.

main.h

#ifndef MAIN_H
#define MAIN_H
#include <ncurses.h>
#include <stdbool.h>
typedef WINDOW Window;
typedef struct {
 Window* W;
 bool* isOccupied;
} GameWindow;
typedef struct {
 int x;
 int y;
} Coord, *CoordPtr;
// Doubly-LL implementation for easy growth
typedef struct CoordLL {
 Coord* loc;
 struct CoordLL* next;
 struct CoordLL* prev;
} CoordLL, *CoordLLPtr;
bool wmvaddch(Window* W, int y, int x, int ch);
int toOneD(int y, int x, int maxX);
bool isOccupied(GameWindow* GW, int y, int x, int xMax);
void freeGW(GameWindow* GW);
int renderMenu(Window* W, int menuWidth, char* title, char* subtitle, int numOptions, char** options); // returns selected index
#endif /* MAIN_H */

main.c

#include <ncurses.h>
#include <string.h>
#include <stdlib.h>
#include <time.h>
#include <unistd.h>
#include "lib/snake.h"
#include "lib/main.h"
bool wmvaddch(Window* W, int y, int x, int ch){
 int xMax, yMax;
 getmaxyx(W,yMax,xMax);
 if(yMax < y || xMax < x) return false;
 wmove(W,y,x);
 waddch(W,ch);
 return true;
}
int toOneD(int y, int x, int maxX){ return y*maxX + x; }
bool isOccupied(GameWindow *GW, int y, int x, int xMax){
 int ind = toOneD(y,x,xMax);
 return GW->isOccupied[ind];
}
void freeGW(GameWindow* GW){
 free(GW->isOccupied);
 delwin(GW->W);
 free(GW);
}
void updateScore(Window* scoreWin, Snake* S, int difficulty){
 wclear(scoreWin);
 box(scoreWin,0,0);
 wmove(scoreWin,2,1);
 wprintw(scoreWin,"Score: %d",S->len*difficulty);
 wrefresh(scoreWin);
}
// This function is not very general, but could easily be changed as needed
int renderMenu(Window* W, int menuWidth, char* title, char* subtitle, int numOptions, char** options){
 wattron(W,A_REVERSE);
 mvwprintw(W,1,(menuWidth-strlen(title))/2,title);
 wattroff(W,A_REVERSE);
 mvwprintw(W,3,2,subtitle);
 int highlight = 0;
 while(true){
 for(int i = 0; i < numOptions; ++i){
 if(i==highlight){
 wattron(W,A_BOLD);
 mvwprintw(W,5+i,5,"*");
 }
 else
 mvwprintw(W,5+i,5," ");
 mvwprintw(W,5+i,6,options[i]);
 wattroff(W,A_BOLD);
 }
 int choice = wgetch(W);
 switch(choice){
 case KEY_DOWN:
 if(highlight < 2) highlight++;
 break;
 case KEY_UP:
 if(highlight > 0) highlight--;
 default:
 break;
 }
 refresh();
 // wgetch 10 -> enter -> no more rendering
 wrefresh(W);
 if(choice==10) break;
 }
 return highlight;
}
int main(){
 srand(time(NULL));
 initscr();
 noecho();
 curs_set(0);
 cbreak();
 int yMax, xMax, menuHeight=10, menuWidth=40;
 float relSize = 1.5; // ~1/3 of terminal should be border
 getmaxyx(stdscr,yMax,xMax);
 if(yMax <= menuHeight || xMax <= menuWidth){
 printw("Terminal window too small!");
 getch();
 endwin();
 return -1;
 }
 Window *menuwin = newwin(menuHeight,menuWidth,(yMax-menuHeight)/2,(xMax-menuWidth)/2);
 keypad(menuwin,TRUE);
 refresh();
 box(menuwin,0,0);
 char* difficulties[3] = { "Easy", "Medium", "Hard" };
 int choice, highlight = renderMenu(menuwin,menuWidth,"Snake","Select difficulty:",3,difficulties);
 delwin(menuwin);
 int boundY = yMax/relSize;
 int boundX = xMax/relSize;
 int borderTB = (yMax-boundY)/2;
 int borderLR = (xMax-boundX)/2;
 GameWindow *gamewin = calloc(1,sizeof(GameWindow));
 gamewin->W = newwin(boundY,boundX,borderTB,borderLR);
 gamewin->isOccupied = calloc(boundX*boundY,sizeof(bool));
 keypad(gamewin->W,TRUE);
 refresh();
 box(gamewin->W,0,0);
 Snake* S = newSnake(boundX-1,boundY-1);
 gamewin->isOccupied[toOneD(S->first->loc->y,S->first->loc->x,boundX-1)] = true;
 renderSnake(gamewin->W,S);
 placeFood(gamewin,S);
 // Difficulty level determines delay
 int usDelay;
 switch(highlight){
 case 0:
 usDelay = 500000;
 halfdelay(5);
 break;
 case 1:
 usDelay = 300000;
 halfdelay(3);
 break;
 case 2:
 usDelay = 100000;
 halfdelay(1);
 break;
 }
 bool collided = false;
 clock_t t;
 double secsElapsed;
 int usElapsed;
 Window *scoreWin = newwin(4,14,yMax-3,xMax-14);
 refresh();
 updateScore(scoreWin,S,highlight+1);
 while(!collided){
 t = clock();
 flushinp();
 choice = wgetch(gamewin->W);
 t = clock() - t;
 secsElapsed = ((double)t)/CLOCKS_PER_SEC; // seconds
 usElapsed = (int)(secsElapsed*1000000); // microseconds
 if(choice == ERR) choice = S->lastDir;
 else usleep(usDelay-usElapsed);
 collided = moveSnake(gamewin,S,choice);
 renderSnake(gamewin->W,S);;
 updateScore(scoreWin,S,highlight+1);
 }
 cbreak();
 freeGW(gamewin);
 clear();
 refresh();
 updateScore(scoreWin,S,highlight+1);
 Window *endGameWin = newwin(menuHeight,menuWidth,(yMax-menuHeight)/2,(xMax-menuWidth)/2);
 keypad(endGameWin,TRUE);
 refresh();
 box(endGameWin,0,0);
 char* endGameOptions[4] = { "Yes", "No" };
 choice = renderMenu(endGameWin,menuWidth,"End of game :(","Play Again?",2,endGameOptions);
 if(choice == 0) main();
 delwin(endGameWin);
 delwin(scoreWin);
 endwin();
}

The snake functions included in the snake file, I think, have fairly descriptive names.

snake.h

#ifndef SNAKE_H
#define SNAKE_H
#include "main.h"
typedef Coord Boundaries; // Boundaries can be summarized by the right-most, bottom-most coordinate
typedef struct {
 CoordLL* first; // Head
 CoordLL* last; // End of tail
 Boundaries* bounds; // Not really a snake properties, but convenient and unchanging
 Coord* foodLoc;
 int lastDir;
 int len;
} Snake;
// Movement functions return true if collision occurs
bool moveSnake(GameWindow* GW, Snake* S, int choice);
Snake* newSnake(int xMax, int yMax);
void delSnake(Snake* S);
void renderSnake(Window* W, Snake* S);
void placeFood(GameWindow* GW, Snake* S);
void growSnake(Snake* S, int newY, int newX);
#endif /* SNAKE_H */

snake.c

#include <stdlib.h>
#include <stdio.h>
#include <ncurses.h>
#include "lib/snake.h"
void renderSnake(Window* W, Snake* S){
 CoordLL* first = S->first;
 while(S->first){
 wmvaddch(W,S->first->loc->y,S->first->loc->x,ACS_DIAMOND);
 S->first = S->first->next;
 }
 S->first = first;
 wrefresh(W);
}
bool reachingFood(Snake* S, int dir){
 switch(dir){
 case KEY_UP:
 return S->foodLoc->x == S->first->loc->x &&
 S->foodLoc->y == (S->first->loc->y-1);
 break;
 case KEY_DOWN:
 return S->foodLoc->x == S->first->loc->x &&
 S->foodLoc->y == (S->first->loc->y+1);
 break;
 case KEY_LEFT:
 return S->foodLoc->x == S->first->loc->x-1 &&
 S->foodLoc->y == (S->first->loc->y);
 break;
 case KEY_RIGHT:
 return S->foodLoc->x == S->first->loc->x+1 &&
 S->foodLoc->y == (S->first->loc->y);
 break;
 }
 return false;
}
void placeFood(GameWindow* GW, Snake* S){
 Coord *foodLoc = calloc(1,sizeof(Coord));
 if(!foodLoc){
 printf("Allocation of food coordinate failed!");
 exit(1);
 }
 do{
 foodLoc->x = (rand() % (S->bounds->x-1)) + 1;
 foodLoc->y = (rand() % (S->bounds->y-1)) + 1;
 } while(GW->isOccupied[toOneD(foodLoc->y,foodLoc->x,S->bounds->x)]);
 GW->isOccupied[toOneD(foodLoc->y,foodLoc->x,S->bounds->x)] = true;
 S->foodLoc = foodLoc;
 wmvaddch(GW->W,foodLoc->y,foodLoc->x,ACS_DIAMOND);
}
bool moveSnake(GameWindow* GW, Snake* S, int choice){
 bool eating = reachingFood(S,choice);
 int xShift, yShift, newX, newY;
 xShift = yShift = 0;
 if(S->lastDir == KEY_UP && choice == KEY_DOWN) choice = KEY_UP;
 if(S->lastDir == KEY_DOWN && choice == KEY_UP) choice = KEY_DOWN;
 if(S->lastDir == KEY_LEFT && choice == KEY_RIGHT) choice = KEY_LEFT;
 if(S->lastDir == KEY_RIGHT && choice == KEY_LEFT) choice = KEY_RIGHT;
 switch(choice){
 case KEY_UP:
 yShift = -1;
 break;
 case KEY_DOWN:
 yShift = 1;
 break;
 case KEY_LEFT:
 xShift = -1;
 break;
 case KEY_RIGHT:
 xShift = 1;
 break;
 }
 newX = S->first->loc->x + xShift;
 newY = S->first->loc->y + yShift;
 if(newX > 0 && newY > 0 && newX < S->bounds->x && newY < S->bounds->y
 && (eating || !isOccupied(GW,newY,newX,S->bounds->x))){
 if(!eating){
 mvwprintw(GW->W,S->last->loc->y,S->last->loc->x," ");
 GW->isOccupied[toOneD(S->last->loc->y,S->last->loc->x,S->bounds->x)] = false;
 S->first->prev = S->last; // Reusing tail memory as head
 S->last = S->last->prev;
 S->first = S->first->prev;
 S->first->prev = S->last->next = NULL;
 S->first->loc->y = newY;
 S->first->loc->x = newX;
 }
 else{
 CoordLL* newHead = calloc(1,sizeof(CoordLL));
 Coord* newCoord = calloc(1,sizeof(Coord));
 if(!newHead || !newCoord){
 printf("Allocation of head or head's coordinate failed!");
 exit(1);
 }
 newHead->loc = newCoord;
 newHead->loc->x = newX;
 newHead->loc->y = newY;
 newHead->next = S->first;
 S->first->prev = newHead;
 S->first = S->first->prev;
 S->first->prev = NULL;
 S->len++;
 placeFood(GW,S);
 }
 GW->isOccupied[toOneD(newY,newX,S->bounds->x)] = true;
 // Unfortunately clunky for now
 switch(choice){
 case KEY_UP:
 S->lastDir = KEY_UP;
 break;
 case KEY_DOWN:
 S->lastDir = KEY_DOWN;
 break;
 case KEY_LEFT:
 S->lastDir = KEY_LEFT;
 break;
 case KEY_RIGHT:
 S->lastDir = KEY_RIGHT;
 break;
 }
 return false;
 }
 return true;
}
Snake* newSnake(int xMax, int yMax){
 Snake *S = calloc(1,sizeof(Snake));
 if(!S){
 printf("Allocation of snake failed!");
 exit(1);
 }
 Boundaries *b = calloc(1,sizeof(Boundaries));
 if(!b){
 printf("Allocation of boundaries failed!");
 exit(1);
 }
 b->x = xMax, b->y = yMax;
 S->bounds = b;
 CoordLL* first = calloc(1,sizeof(CoordLL));
 if(!first){
 printf("Allocation of coordinate array failed!");
 exit(1);
 }
 first->next = first->prev = NULL;
 Coord* coord = calloc(1,sizeof(Coord));
 if(!coord){
 printf("Allocation of coordinate failed!");
 exit(1);
 }
 first->loc = coord;
 // Snake will start in the center, moving right, and with length 1
 first->loc->x = (xMax+1)/2;
 first->loc->y = (yMax+1)/2;
 S->first = S->last = first;
 S->lastDir = KEY_RIGHT;
 return(S);
}
void delSnake(Snake* S){
 CoordLL* temp;
 while(S->first){
 temp = S->first;
 S->first = S->first->next;
 free(temp);
 }
 free(S->bounds);
 free(S);
}

Because it might be easier to read there, I'll include a link to the git repository I've worked on the project within.

Answer 1

Nice first question.

I will not cover anything that B. Wolf has already covered.

The code is nicely indented, some of the variable names and function names are clear. The variables t and S in main(), W and i in renderMenu() and S in reachingFood() deserve more descriptive names.

Inconsistent Coding Style

The naming of functions and variables are inconsistent, they should all use one convention. Use either camelCase or some other convention, but consistent is much better. You are stuck with the names of the functions from ncurses, but all other functions and variables should be consistent. usDelay, secsElapsed, gamewin, isOccupied, collided and highlight are examples of inconsist coding conventions.

Utilize Good Programming Principles

The code is missing out on some basic principals that might help such as Reducing Coupling, SRP and KISS. The functions main(), moveSnake() and possibly newSnake() are too long and overly complex. main() and moveSnake() should be broken up into smaller functions. It is very rare for a function to be 100 or more lines of code.

Avoid Tight Coupling

Avoiding tight coupling has been a programming principle that even dates to before most of the object oriented languages. The function wmvaddch() is defined in main.c but it is used in snake.c, this is a form of tight coupling and should be avoided. Since wmvaddch() is only used in snake.c it should be moved to snake.c and it doesn't need to in snake.h.

Use the Single Responsibility Principle to Reduce Complexity of the Functions

I've been told by some managers that any function over 10 lines is too long. In any case functions should really fit on one screen and anything longer than 30 (an arbitrary view point) lines is too complex. The main() function should generally be a set of functions to perform whatever setup is necessary, execute the program function and perform whatever cleanup is necessary.

The main() function can be broken up into the following functions:

• setupCursesForSnake().
• setupDisplay().
• setupUserLevel().
• playSnake().
• cleanup().

Example of simiplified function:

playSnake(Snake *S, int yMax, int xMax)
{
 bool collided = false;
 clock_t t;
 double secsElapsed;
 int usElapsed;
 Window *scoreWin = newwin(4,14,yMax-3,xMax-14);
 refresh();
 updateScore(scoreWin,S,highlight+1);
 while(!collided){
 t = clock();
 flushinp();
 choice = wgetch(gamewin->W);
 t = clock() - t;
 secsElapsed = ((double)t)/CLOCKS_PER_SEC; // seconds
 usElapsed = (int)(secsElapsed*1000000); // microseconds
 if(choice == ERR) choice = S->lastDir;
 else usleep(usDelay-usElapsed);
 collided = moveSnake(gamewin,S,choice);
 renderSnake(gamewin->W,S);;
 updateScore(scoreWin,S,highlight+1);
 }
 cbreak();
 freeGW(gamewin);
 clear();
 refresh();
 updateScore(scoreWin,S,highlight+1);
 Window *endGameWin = newwin(menuHeight,menuWidth,(yMax-menuHeight)/2,(xMax-menuWidth)/2);
 keypad(endGameWin,TRUE);
 refresh();
}

The previous example would simplify a "Play Another Game" function.

In the function moveSnake() The code within the if clause and the else clause could be functions to simplify the moveSnake() function.

MAGIC Numbers

Because the main function is so long I initially missed the return -1; statement. One suggestion about this statement is rather than using -1, use the symbolic constant EXIT_FAILURE that is defined in <stdlib.h>. While it is not completely necessary, to improve the consistency of the code use return EXIT_SUCCESS at the end of main(). These will make your code clearer and the constants are available on all platforms, the can also be used in C++. The exit(1) in moveSnake() could be exit(EXIT_FAILURE).

In the following code the numbers 0, 1, 2, 5, 3, 1, 500000, 300000 and 100000 are what some professional programmers call magic numbers.

 switch(highlight){
 case 0:
 usDelay = 500000;
 halfdelay(5);
 break;
 case 1:
 usDelay = 300000;
 halfdelay(3);
 break;
 case 2:
 usDelay = 100000;
 halfdelay(1);
 break;
 }

Use symbolic constants for these numbers to make the code easier to read and debug, and in many cases code.

#define EXPERT_DELAY 100000
#define INTERMEDIATE_DELAY 300000
#define BEGINNER_DELAY 500000

A better way to handle 0, 1 and 2 might be to use an enumeration type (enum).

typedef enum {BEGINNER, INTERMEDIATE, EXPERT} PlayerLevel;
PlayerLevel playerLevel = INTERMEDIATE;
 switch(playerLevel){
 case BEGINNER:
 usDelay = BEGINNER_DELAY;
 halfdelay(BEGINNER_HALF_DELAY);
 break;
 case INTERMEDIATE:
 usDelay = INTERMEDIATE_DELAY;
 halfdelay(INTERMEDIATE_HALF_DELAY);
 break;
 case EXPERT:
 usDelay = EXPERT_DELAY;
 halfdelay(EXPERT_HALF_DELAY);
 break;
 }

Avoid Implicit Casting The variable relSize is type float, the right hand side of the lines:

 int boundY = yMax/relSize;
 int boundX = xMax/relSize;

will result in floats and that code is truncating the values of the right hand side. In some instances this might result in the wrong value being assigned to boundY and boundX. The standard C include file includes the functions round(), ceil() and floor() and are more appropriate that the simple truncation here.

Using type double rather than type float will be more accurate.

In any case it is much better to avoid implicit casts and make them explicit:

 int boundY = (int) (yMax/relSize);
 int boundX = (int) (xMax/relSize);

Use exit() Very Carefully The use of the exit() function may bypass cleanup functions. In C++ you can avoid the exit() function by throwing and catching exceptions, in C there is an error handling capability using setjmp() and longjmp(). Generally setjmp() would be used in main() and longjmp would be used where error conditions happening. This stackoverflow.com question discusses setjmp() and longjmp().

The use of exit() should be avoided in programs such as operating systems that are never supposed to end.

The printf() preceeding the call to exit() would be better as an fprintf(stderr, ERROR_MESSAGE);. The printf() function prints to stdout and may not appear in all cases, stderr should appear in all cases.

• mnoronha: Wow, this is all really helpful. Thank you so much!

Answer 2

• Your render function could be re-written as constants, shows that the snake is not changed by this function. e.g.

  void renderSnake(Window* W, Snake const * const S){
   CoordLL* body = S->first;
   while(body){
   wmvaddch(W,body->loc->y,body->loc->x,ACS_DIAMOND);
   body = body->next;
   }
   wrefresh(W);
  }
  

• There is a memory leak around placeFood(). New food is allocated, and there is no call to free() the old food before it is overwritten.

• "clunky" could be reduced to,

   switch(choice){
   case KEY_UP: // dropthrough
   case KEY_DOWN: // dropthrough
   case KEY_LEFT: // dropthrough
   case KEY_RIGHT: // dropthrough
   S->lastDir = choice;
   break;
   }
  

  OR, I suspect that it's only going to be one of these choices reaching this function. Simplify it to just,

  S->lastDir = choice;