Stack Exchange Network

Stack Exchange network consists of 175 Q&A communities including Stack Overflow, the largest, most trusted online community for developers to learn, share their knowledge, and build their careers.

Visit Stack Exchange

New answers tagged

2

Unnecessary code There is no need to make size, nemb even values. if(size % 2) size++; if(nmemb % 2) nmemb++; __get_next_allocation(size_t size)may benefit with rounding up ofsize` Say you want allocations to be a multiple of 8. #define ALLOC_MULTIPLE 8 int __get_next_allocation(size_t size) { if (size % ALLOC_MULTIPLE) { if (size > SIZE_MAX - ...


6

It might be better to treat the credit card number as a string. In the C programming language a string is a null terminated array of type char or character. This would remove all the division in the program to get each character. It would also allow the program to detect if any non-numeric characters were entered. The first test to see if it is valid would ...


0

Allow for Future Modifications and Expansion In the if statements and the while loops it is generally a good idea to create code blocks so that you or others that maintain the code can easily expand the code as necessary. This will also make the code more readable. Examples from the game_loop(game_data* gd) function: if (gd->balls_left) ...


2

The first thing that strikes me is that you're writing error and status messages to the standard output stream, when usually these go to stderr, the standard error stream. I'm not sure it makes much sense to attempt to create an empty file if the file opening fails - why not leave this until it's time to save? What if the reason it failed is because the ...


2

I don't see a great need to re-organise the structure of the code, other than to eliminate the unnecessary global variables (most can be moved to function scope). There are a few portability problems that need to be addressed. I'll start with the assumption about the width of integer types: typedef char byte; // 1 byte \ 8 bit typedef ...


0

Don't Hide the Use of Macros Currently it is unclear in the main program that you are calling macros rather than functions in this line res = hashtable_get(ht, "john", Entry, bucket);. It is not really clear why you are using a macro rather than a function. There does not seem to be a real benefit to using a macro over a function here. Anything that needs ...


5

malloc I start with one of the most common ones. Instead of char *board = malloc(sizeof(char) * BOARD_SIZE) write char *board = malloc(sizeof(*board) * BOARD_SIZE). If you decide to change the type in the future, you don't have to change at more than one place. And besides, sizeof(char) is ALWAYS 1. But the biggest problem is that you're not checking the ...


3

In this function you declare an array of int: int* createList(int quantity) { int list[quantity]; /* <- here */ printf("Please enter a list of numbers\n"); for(int i = 0; i < quantity; i++) { scanf("%i", &list[i]); } return list; } /* list is free'd here, so you return a pointer to free'd memory */ Accessing the ...


2

Here are some things that may help you improve your program. Don't hardcode file names Generally, it's not a good idea to hardcode a file name in software. Instead, it would be better to allow the user of the program to specify the name, as with a command line parameter. Don't use assert to handle errors The use of assert is to help debug the program, ...


2

matching_brace_position does not return -1 as advertised (if the matching brace is not found). If the matching brace is indeed not found, it returns nothing (in fact, it would access source out of bound). This is UB. handle_loop_end assumes a well-formed BF program. With an ill-formed one (having a stray ]) it would try to free something which wasn't ...


2

You're right to be concerned about allocating memory in the function. One obvious problem is that malloc() can return a null pointer, so we need to check for that before we think about using it: uint32_t* c = malloc(sizeof *c * num_chars); if (!c) { return c; } Making two passes over the input can be problematic, as then we have two pieces of code which ...


3

I find it confusing that the header data sometimes contains the data in little-endian and sometimes in native-endian format. To avoid this, I would rather define: typedef struct { uint8_t a, b, c, d; } uint32_le; void uint32_le_set(uint32_le *var, uint32_t value) { ... } uint32_t uint32_le_get(const uint32_le *var) { return ...; } That way you cannot ...


4

Barring any compiler heroics, you are computing n*n a total of \$n^3\$ times. You might want to cache that result. const int nn = n*n; B[j + n_times_i] is a linearly increasing address location, given that j increases by 1 for each middle loop, and and i increases once for each outer loop, which is n increases of j. Taking advantage of that, you can skip ...


2

Overall, this is well-written code. One thing I would change besides what has already been suggested by others is your call to system("clear") which creates an OS-specific dependency. You can instead use #ifdef to check the platform and then call the appropriate command to clear the screen. Additionally, this can be wrapped in a function to allow for code ...


1

The indentation seems to have lost a level for much of the code, which makes it hard to read. I'm guessing something went wrong when you copied the code into the question, and that your source doesn't really look that bad. Let's start with the definition of main(). It's generally better to declare main as a function taking no arguments: int main(void). ...


7

The following remarks may be nitpicks. Since your code is already quite good, it's all I have to say. :) #include <stdio.h> #include <stdlib.h> #include <stdbool.h> Since all the above headers are from the standard C library, they should be in alphabetical order. When you include other headers like <sys/type.h>, the order is ...


10

There's two things I'd change to simplify your code: Use a two dimensional array. Sure you can do the conversion from a 2d position to 1d easily, but the compiler can do and board[1][1] is rather more obviously the middle of the board than board[4]. Instead of hardcoding your logic for what positions you have to check, think about a programmatic approach ...


6

The code looks clean and readable. You could rename check to checkForWin to tell what it checks. I would also suggest to improve that procedure by declaring the lines as a constant array instad of writing a similar test 8 times: const int lines[8][3] = { { 0, 1, 2 }, // rows { 3, 4, 5 }, { 6, 7, 8 }, { 0, 3, 6 }, // columns { 1, 4, 7 },...


9

I think youre code is quite nice. Easy to read and the tasks are nicely splitted into functions. So theres not much to add. Theres just one thing bothering me. Don't omit curly braces. This: // Check rows if (b[0] == p && b[1] == p && b[2] == p) return true; if (b[3] == p && b[4] == p && b[5] == p) return true; if (...


18

First of all: nice work! It's easy to read and understand. Program organization It's very good that you split the task to small functions. Reading the body of main reveals nicely the overall flow. Ideas for further improvement: place does two things: it reads input from user and updates the state of the board. It would be good to separate these logically ...


4

this is a performance-critical function any replacements will need to keep this requirement in mind Use restrict fusc_word(ulong u, ulong* a, ulong* b) uses a, b and certainly the algorithm does not work if those pointers point to overlapping data. A complier can not make that assumption though and so must emit code as if a and b potentially point to the ...


1

Don't Re-Invent the Wheel Unless Absolutely Necessary The C programming language already contains the function strcpy(destination, source). You have access to this function and many more string functions when you include string.h in your program. There is also the char* strncpy(char* dst, const char* src, size_t size); function that limits the number of ...


3

EDIT: I also fixed a typo: main should be int main(void)! After fixing the bugs @chux found, there was still another bug: This line: tmp -= (tmp - min_len + 3) / 4; should be: tmp -= tmp / 5 - !(tmp % 5); I also restructured the big function into smaller functions, and used CHAR_BIT instead of the magic number 8: /* 2019 - Alejandro ...


3

Use Symbolic Constants Rather Than Numeric Constants In most programming languages there is a way to define symbolic constants for numbers which makes the code more readable and easier to maintain. When raw numbers are used in code they are sometimes called Magic Numbers. Using Magic Numbers is generally considered a poor programming practice as discussed in ...


3

Since the function is hardwired to 3x3 matrices, it would make sense to pass a 3x3 matrix as an argument: int formingMagicSquare(const int s[3][3]) This way you will not worry about what to do if s_rows != 3. stdlib provides int abs(int). The line if(cost4_this_operation < cost)cost = cost4_this_operation; better be split into if(...


Top 50 recent answers are included