44

PEP 8 PEP 8 recommends that constants be written as "all capital letters with underscores separating words.". So something like: yellow = (255, 255, 0) Should be: YELLOW = (255, 255, 0) or possibly an enum (more on this later) Inconsistent quotations Usually a project will stick with either "" or '' unless you have particular reason not to.1 But for ...


30

Unless you know how long the game loop will be on every computer, making your sleep a constant is generally bad practice. If you know that you want 2fps, a good way to keep it in line is get the time at the start of the game loop, then at the end, find out the difference, and use that to calculate the amount of time needed to sleep to keep the step the same. ...


28

1. Introduction This is not bad overall, considering that this is your first program written with PyGame. I've made many comments below, but don't take the length of this answer to heart: there are always many things to say about a piece of code of this length. 2. Game design issues The game could do with some instructions. I had to look at the source ...


24

Object Usage This code: Game game = Game(); Game().Run(); cout << "Game has terminated successfully, score: " << game.GetScore() << ", size: " << game.GetSize() << endl; ...isn't doing what I'm pretty sure you think it is. This part: Game game = Game(); creates an object named game which is of type Game. But, I'd prefer ...


22

This review is going to be mostly on code style and general code quality improvements. OOP: First consideration is that for a C++ program we expect to see some Object Oriented Programming - OOP. Your program is basically structured programming, which look a lot more like C than C++. You should start out by refactoring the code into a few classes. Some ...


22

In addition to another answer I will add these places to improve (I will not repeat all things from that answer, my answer is just an addition): 1. Code inconsistency You have several places in your code where your code is inconsistent def collision(): ... return 1 return 0 def wall_collision(): ... return True return False Both ...


19

if(b) return false; else return true; With the ! operator, you can negate a boolean value. So by saying !b, you can swap the cases around: if(!b) return true; else return false; But then you have "if it is true that b is not true, return true, else, return false". At that point, you might as well return "...


17

It's pretty good for a first game. There are a few things I would change: 1. Dependencies You use jQuery for two purposes in your code: $(document).ready to trigger the game setup, and $(document).keydown to catch the keyboard events. However, there really isn't any need to use jQuery for either of these things, so all it does is add to the load time of ...


17

Things that could be improved: Portability: Every time you add an #import to the top of your C file, you potentially create a dependency. For example: #include <windows.h> creates a dependency that the program can only be compiled on a Windows system. #include <stdio.h> #include <time.h> #include <stdlib.h> #include <conio.h>...


17

Just a few remarks. Reviewing bottom-up. public class MultipleValuesDictionary<key, value> Don't name classes dictionary if they aren't one (implementing the IDictionary<,> interface). It's confusing. You can derive a type from the dictionary instead of implementing only parts of it and inventing new vocabulary for it. Don't come up with new ...


15

using namespace std; is a bad practice. I'm glad to see you didn't use it in the header. Better still to not to use it at all. Please see this post for more information. int main(int argc, char * argv[]) If you are not going to use the command line arguments anyway then use the empty parameter main: int main() return 0 at the end of main is unnecessary ...


14

I sense that you think too much about SOLID, padawon. You need to think about what the game IS, and the things that ARE. Objects do things. What they do defines what they are. Properties capture the state of a thing as it does stuff. Leave the search for the unified field theory of object oriented design for later. Obsesssing over Open/Close results in one-...


13

Using constants Rather than using #define and creating a bunch of macros like this: #define PLANEHEIGHT 20 #define PLANEWIDTH 50 #define INITIALSNAKELENGTH 3 #define FPS 5 You should use const instead, like this: const int PLANE_HEIGHT = 20; const int PLANE_WIDTH = 50; const int INITIAL_SNAKE_LENGTH = 3; const int FRAMES_PER_SECOND = ...


13

Some thoughts on your general code style (some points might depend on personal preference): I suggest splitting HTML/CSS/JS into different files Your use of indention and whitespaces is inconsistent function launchFullscreen(element) { if(element.requestFullscreen) { element.requestFullscreen(); has an indention of two spaces snake.game = (...


11

Beyond the SOLID principles, think about your classes and methods. Classes/objects are things that exist, methods are things they do. So when naming them, class names should be nouns, methods should be verbs. If you find yourself naming your classes after verbs, it indicates that you aren't thinking through what those classes are actually for. As such, I ...


11

Well, this is my first code review and by no means I'm an C# expert or something like that, but anyways I'd like to give a general opinion about your code. First and foremost, you're using C# which is mostly an OO language, so I encourage you to write your code using OO constructs. In this case, it means that you should create classes to abstract the things ...


10

The "annoying blinking" is probably caused by your calling system("cls"); at the top of your Boarder() function, which you call repeatedly from inside your Move() function. It would blink less if you didn't clear the screen every time your redraw the border, and/or if you didn't redraw the border for every move.


10

First of all congratulations for this little entertaining console game. It is simple but entertaining. I felt like I was back in the old mobile games era. I don't have the time to rewrite all of the code but I still want to give some hints for improvements. Here are some random observations: Don't use global variables, they are a maintenance hazard. ...


10

size: int ... self.size = 2 You never read size, and Python Lists already know their own length in case you did need it. You're just duplicating functionality that List already gives you by manually keeping size in sync with the list length. Separate game logic from screen rendering details You keep snake coordinates scaled by pixels. This seems to ...


9

From a once over: Good I like how you use an IIFE I really like how you use direction = keys[event.keyCode]; Not so good You are not consistently applying the 2nd good technique, for example this: function setWay(direction) { switch(direction) { case 'left': if(old_direction!='right') { ...


9

Coding style Please follow PEP8, the style guide of Python. The code will become so much easier to read. More tuple-assignments please Use tuple-assignments more aggressively. For example, instead of this: x1 = self.followers[-1].coords()[0] y1 = self.followers[-1].coords()[1] x2 = self.followers[-1].coords()[2] y2 = self.followers[-1]....


8

Welcome to Code Review! There's lots that can be improved about this code, so lets get started. Use the Random class instead of Math.random() Instead of this Math.random() * RANDOMPOSITION Do this int randomNumber = this.random.nextInt(RANDOMPOSITION) The reason for this is that random.nextInt is more efficient and less biased (see here https://...


8

I'm not really the best at reviewing code (I have my own failures in coding too), but I have a few things that I'd like to point out and share. Consider setting up the new Snake object with an __init__ instead (unless you intend to reuse the same Snake object just to create a 'new' snake) You may wish to just move the creation of the Snake body object to ...


8

Code review I like the name "Snake on a plane" and was hoping to see some wings and the like. Maybe that's to come. Not bad if this is your first game. I have gone over it from top to bottom and jotted down points as I found them. Some points Don't timeout requestAnimationFrame because you will get inconsistent frame rates. Use a frame counter and skip ...


8

Some additional points: Avoid "God classes". Your Game class does absolutely everything. This makes it hard to see which member variables are used where, and is one step away from using global variables. The larger the program, the harder it is to understand. Classes should follow the Single Responsibility Principle (SRP), and be responsible for only one ...


8

Here are some things that may help you improve your code. Don't declare enum const In snake.h, the direction enum is declared as const but this is an error, since only functions and objects can be declared const. Use const where practical The Snake::getSnakeSize() doesn't alter the underlying Snake and so it should be declared const. Additionally, I'd ...


8

Your program is definitely improved over the last version. Good job! Here are some ideas for you about further improvements. Make the object interface easy for the user The app object has two public functions, start and end that are apparently intended to be called in that order. To me, it would make more sense to eliminate end and simply move the ...


7

I don't know SFML at all, so very partial review. auto random(const std::uniform_real_distribution<float>& dist) { static auto& RandomEngine = randomEngine(); return dist(RandomEngine); } There are two portability bugs here: randomEngine() returns a temporary, but auto& will deduce a non-const reference type here - that's an ...


7

Oh boy! A snake game. I'm excited, since this was also one of my first games. So let's get to work, shall we? Keep it short and simple The biggest concern with your code gets obvious if we just have a look at your post. Your whole code is inside a single file, which makes it hard to navigate. Your file ist "just" 270 lines though, but still, it's a first ...


7

A few recommendations per class: Food.java What's the point in this class? It just's just a Point with a constant colour. I would just have a FOOD_COLOUR constant in your Painter and then you can scrap this. GameLoop.java frameRate and interval should be final. Actually you don't need frameRate at all because you don't use it for anything other than the ...


Only top voted, non community-wiki answers of a minimum length are eligible