Hot answers tagged

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 "...


15

Minimize math operations if (((double)after.tv_sec*1000000 + (double)after.tv_usec)-((double)before.tv_sec*1000000 + (double)before.tv_usec) > timer){ //time difference in microsec accuracy You do four conversions from integer types to a double precision floating point type. And do two multiplications times a million. Consider //...


13

Avoid numbered variables char space_1 = ' '; char space_2 = ' '; char space_3 = ' '; char space_4 = ' '; char space_5 = ' '; char space_6 = ' '; char space_7 = ' '; char space_8 = ' '; char space_9 = ' '; int space_1y, space_1x; int space_2y, space_2x; int space_3y, space_3x; int space_4y, space_4x; int space_5y, space_5x; int space_6y, space_6x; int ...


12

Here are a number of things that may help you improve your program. Eliminate global variables where practical Having routines dependent on global variables makes it that much more difficult to understand the logic and introduces many opportunities for error. For this program, it would be easy and natural to wrap nearly all of the global variables in a ...


12

Throttling Your program runs in a tight loop, consuming all the CPU it can get (and battery power, if on a mobile device). For a clock that a user might want to run in the background for a long time, conservation is important. Inserting even just a 0.1 second sleep between updates brings CPU usage from 95% to 1% on my machine. "Private" methods close() ...


11

Some style nitpicks... In your initialization you could replace things like: 'five_minutes': [ None, None, None, None, None, None, None, None, None, None, None ], with the arguably much more readable: 'five_minutes': [None] * 11, You are very often repeating the pattern int(floor(a / b)), where both a and b seem to be guaranteed to always be ...


8

Capitalize your defines #define num_spaces 9 should have NUM_SPACES instead of num_spaces. Identify your local functions Tell the compiler when you don't intend to export functions to a different translation unit. Mark all of your functions static except main. Const integral args aren't needed This: int ai_fart(const int chance_to_fart); doesn't ...


8

I can't run this unfortunately. curses seems to have issues on Windows. I'll just focus mainly on style and design. There's a few notable things about this chunk: # I also exit the loop if one of the following conditions if verified if ((ch == curses.KEY_UP and j > 0) or (ch == curses.KEY_DOWN and kcell == False) or (ch == curses.KEY_LEFT and (i != 0 or ...


7

I've made this Community Wiki because my answer revolves around 1 example that those more knowledgeable about C could probably improve and expand. Improvements are welcome! Practice DRY code There's one piece of feedback you received a bit in your previous review, and I think taking it to heart would improve your coding style tremendously: practice DRY ...


7

A small review I noticed in both iterations of your code that you have to forward declare every single function. This is not advisable. It is better to declare main() last, thereby eliminating your need to forward declare. After all just imagine the forward declarations required for a project of significant size. After that you might also want to start ...


7

Besides of that I support the idea of having constant values like window's height, width and input_row settings as class constants (uppercase names) and referencing negative kcell flag as not kcell instead of kcell == False here's some list of advises in terms of better code organizing, restructuring conditionals and eliminating duplicates: window's half of ...


6

I see a number of things that could probably help you improve your program. Eliminate unused variables In main(), argc and argv are unused. I'd recommend either eliminating them by changing the function to int main() or better yet, use them to pass in the required initialization variables for dimensions and seed value. Use standard function where ...


6

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 ...


6

Compiling using clang with warnings turned on reveals a number of issues: $ clang -Wall -lcurses -o ttt ttt.c ttt.c:240:1: warning: control may reach end of non-void function [-Wreturn-type] } ^ Why is main_menu() calling itself recursively? If you want a loop, write a loop. ttt.c:339:1: warning: control may reach end of non-void function [-Wreturn-type] ...


6

This is a pretty nice effort for a C++ beginner. Well done! I see a number of things that may help you improve your code. Don't reseed the random number generator more than once In the Field::place_food() routine, the loop is written like this: while(true) { static std::mt19937 rng; rng.seed(std::random_device()()); std::...


5

Running your code through pycodestyle will highlight some issues to make the code more familiar to Pythonistas. Other than that: while 1 should be written as while True (Explicit is better than implicit) Rather than checking for a specific character to quit I would just assume that people know about Ctrl-c Use as easily readable names as possible. ...


5

Code organization The add_first_word() closure passes information via three nonlocal variables, which makes the code hard to understand. I find it dissatisfying that the first word needs to be handled as a special case. Instead, I would recommend using ' '.join(…) to help you add spaces conditionally. That seems especially appropriate, considering that ...


5

Consider following the Single Responsibility Principal and break the function up into smaller functions that do less. Note: This answer makes use of the loopEvos() that is provided in the updated version of the source files on GitHub. void loopEvos (Grid* grid, size_t speed) { nodelay(stdscr, TRUE); // Getting of the input is asyncronous // ...


5

Here are some things that may help you improve your code. Use const where practical The render_screen routine does not alter the passed map, as expected, so it should be passed as const char map[] instead. Eliminate global variables where practical Having routines dependent on global variables makes it that much more difficult to understand the logic and ...


5

Your display function's logic is very hard to follow, there are a lot of special cases. Some comparisons are with COLS-1, others with COLS-2, and it's not clear why. The variable names don't help much here. The last line displayed is odd too, you're not checking for "vertical overflow". You call refresh and manually move the cursor around for each character ...


5

Your shuffling-algorithms is broken: See The Danger of Naïveté for the explanation, and Fisher-Yates shuffle on wikipedia for a working replacement. Aside from that, you really should only initialize the random-generator once, at the start of the program. (For debuggability, you might want to add an option to specify the seed on the command-line.) For no ...


4

Split main into functions Right now your program is 80% contained in one function, main(). This makes the code difficult to read, and is not a good practice. You should be splitting your code into functions, such as: Inputting game parameters Initializing game state Displaying the cards on the screen Displaying messages on the screen Receiving and ...


4

import curses import pygame pygame.mixer.init(frequency=22050, size=-16, channels=2, buffer=4096) player = pygame.mixer.music When giving modules a shorter name, the usual method is from pygame.mixer import music as player It makes it a little clearer what you are doing. song = "testshort.mp3" player.load(song) player.play() For anything but a quick ...


4

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 ...


4

Efficiency In "list" class, you implemented linked list behaviors, which has provides O(N) time complexity for get and add functions. You could use C++ containers, such as vector, which provides constant time for your add/remove/get functions, see push_back, erase, pop_back...functions. Naming The snake class actually represents part of a snake; it is ...


4

These: char space_1 = ' '; char space_2 = ' '; char space_3 = ' '; char space_4 = ' '; char space_5 = ' '; char space_6 = ' '; char space_7 = ' '; char space_8 = ' '; char space_9 = ' '; int space_1y, space_1x; int space_2y, space_2x; int space_3y, space_3x; int space_4y, space_4x; int space_5y, space_5x; int space_6y, space_6x; int space_7y, space_7x; int ...


3

Error handling unsigned long long is guaranteed to hold at least 64 bits. If you are using strtoull(), then limiting yourself to uint64_t is just asking for trouble with no benefit. In fact, your conditional code in #if ULLONG_MAX > UINT64_MAX doesn't even compile: there is no such variable y. What does the return value of my_strto64() represent? ...


3

I'll steer clear of the high-level design questions, and just critique the code itself within its own context. Curly braces: for(int y=0; y < blocksize; ++y) for(int x=0; x < blocksize; ++x) if(b[y][x]) drawsquare(row + y, col + x, b[y][x]); While omitting the curly braces is valid, you should try to avoid doing that if the full ...


3

You can create a block without an if statement. For example, these are equivalent: { // some code } if (true) { // some code } But it's not a good practice to create blocks like this. It's a code smell, suggesting to wrap // some code in a dedicated function instead. Anytime you think some neat trick is "unconventional", it's most probably a bad ...


3

Overall, I'd say this code seems pretty straightforward and is easy to read. My main concern is the performance. You're reading a file in one character at a time and that's going to be slow. Rather than using fgetc(), I recommend using getline() instead. It's a standard function that reads an entire line at a time. The other concern I have is what happens ...


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