Skip to content

Instantly share code, notes, and snippets.

@fbartho
Created April 8, 2018 19:17
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save fbartho/4ef144aa3806804083e4d11f2db35848 to your computer and use it in GitHub Desktop.
Save fbartho/4ef144aa3806804083e4d11f2db35848 to your computer and use it in GitHub Desktop.
noahhaasis/conwaysGameOfLife Feedback on Request

noahhaasis/conwaysGameOfLife Feedback

Notes:

  • Most of my feedback is going to be superficial and possibly wrong!
  • I'm not an active expert in C.
  • I haven't used the SDL toolkit since college.
  • I didn't run this code, I simply looked at it!

README

  • Great presentation, I love the screenshot. -- You could even record some Gifs!
  • SDL library reference could include a link to documentation & instructions on how to install it.
  • Conway`s Game of Life could include a reference link to wikipedia if people are unfamiliar with it.
  • Could include documentation how what target platforms/operating systems this is expected to run on or what yout tested it on.
  • Could include build & run instructions. (possibly with a Makefile?)
  • You could include some sample boards that have known-pretty behaviors (And record gifs of them :) )... Glider-guns, Oscillators, Still lifes, Spaceships, Puffer-breeders that fly while dropping glider guns behind them… Rakes… So many cool patterns!

conways_game_of_life.c

  1. Good addition of a TODO list.
  2. load_from_file -- It doesn't look like you document the filename parameter anywhere else. A comment or "usage" line would be appropriate here. Also you could write it as: bool load_from_file = argc == 2;
  3. Great job using scancode instead of keycodes. I use a dvorak keyboard layout and often have to revert to qwerty for games.
  4. Great job using fprintf(stderr... in most places! One missed spot

board.c

  1. power_of_two Why do you have a power_of_two method? Have you measured your performance yet? I'd be very surprised if you have a workload where you need to inline your own power of two method. For all you know, your compiler might have a better implementation with more powers, and this is statically increasing the size of your file with a trivial performance benefit. That said, if you have measured performance with just pow() vs your inline and that works better on your target compiler & hardware -- then great!
  2. Generate a 32 bit random value -- I think you don't actually want a random 32 bit value. Instead, I think you want a random selection among all possible board coordinates on your board. A research adventure you can dive down is would be to google "uniform random" and "modulo bias". If you're running this on BSD-based operating systems (macOS, iOS, and others) I would tell you to look up arc4random and more specifically arc4random_uniform. This may not be important to your application, and your solution will probably work fine, but I'd thought I'd share. For some standard C solutions that I haven't verified for true correctness: Raw C solution 1 Raw C solution 2.\
  3. Generally, I like the things that are commented in this file. It really clarifies what's going on.
  4. Generally, this file looks well structured and consistently formatted to me. This makes it easier for me to read.
  5. Optional: This is a stylistic debate, but always wrapping conditional blocks in curly braces can be safer when working on a project with multiple developers. if-return
  6. Optional: You could split each clause out into a named boolean variable in this long if-block
  7. Empty-func
@fbartho
Copy link
Author

fbartho commented Apr 8, 2018

Context was: https://twitter.com/haasis_noah/status/982993172174385152

Hello my name is Noah. I'm 16 and trying to improve my C coding skills. I just wanted to ask if you could maybe look at some of my C code and give me a tip where I have to improve. I have a C implementation of the game of live in my repository https://github.com/noahhaasis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment