Console.Clear();
operations made the screen flicker quite a bit. Also, the drawing of the border was a bit long winded. For example, you are on a row at 0, 0.. You can write the entire horizontal bar in one go with one Console.Write('■■■■■■■■■■■■■■');
statement. The same applies for the bottom row of the border.Console.Clear();
we can just create a method named ClearConsole();
that writes black over the inside of the frame.pixelPlaceHolder
or pixelCursor
over something short but harder to read. I know that is a preference. But I've heard fewer people complain about a long var name, then a hard to read name. bool buttonPressed
. So all you have to do is make buttonPressed, true or false. example: const string UP = 'UP'
Reason being, if anything were to change with your program in the future, and 'UP' becomes 'up' then you only have to change it in one spot, and not the whole program. Also 1 could mean anything. Instead, give it a name or a enum value. Like const int GAME_OVER = 1
or enum Condition { Started = 0, GameOver = 1};
This way someone sees the condition and not a number. You can call it like this while(GameState Condition.Started)
or while(GameState != GAME_OVER)
Especially if you are using numbers in more then one spot. It cuts down on confusion for yourself and others. Trust me! Leave your code for a couple of weeks and come back to it.. Let me know if you don't go, 'what was I doing here?' :)Draw()
. It takes in the changes from the switch statement, and redraws the snake based on the input. Then have another function for the other while loop. Call that function UserInput()
. If you want Co locate the switch statement with that loop, or have it in its own function. When you press a button while it's in the loop it can call the switch statement to make the adjustment, and then redraw the environment by calling the draw function. You may find by doing this 'refactoring' that you can rid yourself of a while loop or loops, which will reduce your Big O notation which can make your program faster. Obviously, since your program is so small this reduction in loops will not be that noticeable. If you get in the practice now of doing it, when you move to something bigger, it will make a big difference. PascalCase
for classes, methods, and propetries; and camelCase
for variables. The most obvious culprit is your class pixel
, which should be named Pixel
. It sounds like a trivial thing but it is really important when reading code.Pixel
class then should look something like this:Pixel
cleaner. I also feel the class would be better as a struct
, but let's not go there.var
instead of using the full class name. The latter is a stylistic issue, but var
is more readable, imo.Console
class a lot. With C# 6, you can make life easier for yourself and write using static System.Console;
at the top of your file. And then you don't have to suffix every call to the Console
class with the class name. Less typing. Yay.screenWidth
and screenHeight
variables any more. Personally, I think the less variables flying around the better. YMMV. I will remove them from subsequent code in this review.Pixel
class, then you refer to the body positions and the berry position as lists of numbers ? Why not use the class you already created?Ctrl + .
, then choose Extract method. You can call it DrawBorder()
. Note you dont need to use Console
here at all, too, due to using static
.gameover
as a yes/no value. Don't use an int
for that. Use a bool
. Same thing with buttonPressed
.string
values to indicate direction. These values cannot be other than four particular cases. This is the perfect use case from an enum
type.movement
variable. It becomes :'NODIRECTION'
as a viable direction.DrawPixel(Pixel pixel)
. It takes a pixel as input and draws it.List<Pixel>
. You can define it as: if
statement with an assignment statement (using |=
) that takes less vertical space. It adds up to the same thing, due to how the OR operator works.score
variable for control. I felt it was clever enough so I left it as is.Console.ReadKey();
in the end so the Console app does not quit immediately.while
loop with a condition that is more immediately readable.while
loop into a separate method for readability. else if
.System.Linq
. The list type has its own running tally without calling a method.Stopwatch
instead of crude DateTime.Now
Thread.Sleep(5)
is needed, otherwise loop will go horribly fast and it will eat lots of processorpower.Worksheet_Selection_Change
event. Because I need to store values after exiting the procedure, I couldn't figure out how to implement a Class
.GetAsyncKeyState
API