19
int letter is not a letter. When printing you call it The number. Name it accordingly: int number.
The condition (1 <= letter && letter <= 26) == false is very hard to follow. As a general rule, avoid boolean constants in conditions. Rewriting it as:
!(1 <= letter && letter <= 26)
immediately calls for a deMorgan transformation ...
14
Use enums to give names to numbers
It would be great if you could write PAWN instead of 1, since it will be much clearer what you are doing in the code. The way to do this is to declare an enum for all possible chess piece types:
enum Type {
NONE = 0,
PAWN,
ROOK,
BISHOP,
KNIGHT,
QUEEN,
KING,
};
I added NONE as well, it will be ...
13
Bug: When you feed an empty file to your program, it ends up in an endless loop.
Bug: the 24th letter of the English alphabet is X, not S.
Instead of const char * you should rather declare const char alphabet[], to make the code match the comment above it. Don't confuse strings and pointers to strings. The authors of the cs50 library do that, and they do ...
10
See
this Stackoverflow answer for
a discussion on the optimal growth factor for dynamic
arrays. The gist of it is that it depends, but growth
factors around the golden ratio are easier for the memory allocator to
handle. I'd recommend using 1.5 because it is easy to implement:
DEST->cap += DEST->cap / 2
It likely fares worse on artifical benchmarks ...
10
It's generally best to divide your code into separate sections (possibly separate functions) for argument handling and actual computation.
Errors should be printed to standard error rather than standard output. Also, prefer small integer values for exit status (and since we're in main(), we can use simple return rather than exit() - note the useful ...
9
About aesthetics
I used the "long" form of all math statements, avoided the use of pointers, and used only while loops for aesthetic purposes (simply put, I think the code is prettier when it is written like that.)
While making the code look aesthetically pleasing can be beneficial (especially if it makes it more readable), you should not give that a ...
8
It's a very unusual style to begin identifiers with uppercase letters.
Include <stdbool.h> and use the bool type for flags.
Many of the variables can be moved to smaller scopes.
I think you should use for loops where appropriate - grouping the initialisation, predicate and advance clauses together improves clarity.
Obviously this code lacks the ...
8
Use getaddrinfo()
getnameinfo() has a counterpart: getaddrinfo(). Prefer to use that instead of having to call inet_pton() multiple times. You can force it to only allow numerical IP addresses as input. Here is how it would work:
struct addrinfo *ai;
struct addrinfo hints = {
.ai_flags = AI_NUMERICHOST,
.ai_family = AF_UNSPEC,
};
int res = ...
7
We're missing an include of <stddef.h> to define size_t (or any of the other headers which define it).
We're missing an include of <stdlib.h>, needed for malloc() and friends (and this would define size_t for us, too).
With those fixed, and sufficient minor changes to the test program, I managed to compile with only a few warnings.
DEST = ...
7
Since command line arguments are null-terminated, we can avoid strlen() entirely by just running through the string until we hit '\0'; this also gets rid of contin and i
(*p > 0x2F && *p < 0x3A) isn't very obvious, I'd just use '0' and '9' to make it easier to understand the intention and to be consistent with the rest of the code
You could use ...
6
Good first effort. Coming from Java, you'll find memory management in C new and frustrating:
Memory management
You can't just call malloc and directly use the result like this:
Hand player_hand = malloc(sizeof(Card*) * MAX_CARDS);
for (int i = 0; i < MAX_CARDS; i++) {
player_hand[i] = NULL;
}
If malloc() fails, it will return a null pointer, and ...
5
Your code uses an inconsistent style for its spacing. Sometimes you write if (cond) and sometimes if(cond). You should pick either style and stick to it.
I am assuming that you prefer a condensed writing style because in several cases you don't even leave a space after a comma. In that case, you should consistently apply this condensed style, and your code ...
5
Portability
While #pragma once is widely supported it is not part of the C programming standard. To make this library more portable, use include guards such as shown in the example below.
Ease of Use
Don't force the users of the library to include files in the proper order, functions that are written using other functions in the library need to include ...
5
has a time complexity of O(n^2).
I think you are incorrectly using "n" as if it is the size of the n×n matrix.
The "n" in O() notation refers to the total size of the input data.
So, for a m×m matrix, the "n" value for complexity would be m2.
Your algorithm is O(n).
This code doesn't use extra memory
Is there a specific reason for doing the swap ...
5
Don't Return Values from void Functions
if (!A || !B)
return false;
else if (A == B)
return false;
The function void swapNode(List *listNode, char nameA[30], char nameB[30]) is declared void, which means it doesn't return a value, yet it attempts to return false in two places. Some compilers actually report this as an error. Rather ...
4
Use Doxygen to document your code
It's already mentioned by others, but you have a lot of comments. It's good to document all the functions, but you are not using a standard code documentation language as far as I can tell. I recomment that you use Doxygen. The advantage is that it understands C and C++, and will perform checks on your documentation, such ...
4
Small review
Put test first
int getValidMoves(int x, int y){
// Move test here
if(x > 7 || x < 0 || y > 7 || y < 0) {
return 1;
}
int piece = board[y][x];
int color = blackWhite[y][x];
int canMove = 1;
moves[y][x] = 2;
// Why test so late?
// Damage has been done access the above arrays with out-...
answered Dec 26 '19 at 11:56
chux - Reinstate Monica
15.9k2 gold badges18 silver badges50 bronze badges
4
Design
Why do you need a create_head_node() that takes a node as the first parameter? Seems counter intuitive. Personally I would just remove that function does not seem needed to me.
The other thing is that you don't seem to have a way to initialize an empty list (yes this can simply be the NULL pointer but it does leave it easy to accidentally have an ...
4
How can I decrease code length?
You could split your big for loop into 2 smaller ones. The first one would print all diagonals up to the main one, and the other one the remaining ones.
For instance, let's say there's a matrix like:
1 2 3
4 5 6
7 8 9
Then, your first loop would print 1 4 2 7 5 3 and the second one 8 6 9. The code would look conciser and ...
4
Recovery from Errors
The safe_malloc() function looks good, however, you could use setjmp and longjmp rather than exit(EXIT_FAILURE) to attempt to recover enough to clean up after errors occur and to only exit the program from main. According to an answer on this stackoverflow question they can also be used for co-routines. The additional information that ...
3
Put const in the right place
You write int32_t *const fdMap, which means the pointer fdMap is constant, but it allows writes to the values inside the map. You probably meant that you want the contents of fdMap to be read-only, so you have to write instead:
const int32_t *fdMap
However, if you want you can make both the pointer and the values pointed to ...
3
On Windows, you'd rather use the console API directly instead of stdio.h. This should increase performance somewhat, since these functions are what getchar etc will end up calling anyhow.
Other issues:
All functions should be declared to take void as parameters, rather than to accept any parameter. If they are properly inlined this should hopefully not ...
3
regarding:
void rotateMatrix(_matrix,_size)
do NOT use leading underscores for variable and/or parameter names.
a function signature must have the variable types as part of the signature.
when compiling, always enable the warnings, then fix those warnings
regarding:
main(argc,argv)
there are two valid signatures for the main() function, the are:
...
3
Here, we print an error message to the standard output stream:
puts("Incorrect input.");
I'd expect to use standard error here:
fputs("Incorrect input.\n", stderr);
(Note that puts() appends a newline, but we have to provide our own for fputs().)
Don't use assert() for run-time checks. assert() compiles to nothing in non-debug builds, so we risk ...
3
Here are some things that may help you improve your program.
Prefer const to #define
For constants such as TOTAL_BITS, it's generally better to use a named const value rather than a #define. The primary reason is that the const definition enforces type checking, while the macro does not. However, see the next suggestion.
Use sizeof for portability
The ...
3
I get quite a lot of warnings when compiling with a reasonably picky compiler¹. Many are due to assigning string literals (const char*) to char* variables, which risks attempting invalid writes. For example:
236002.c: In function ‘main’:
236002.c:172:30: warning: passing argument 3 of ‘insertHead’ discards ‘const’ qualifier from pointer target type [-...
2
Firstly, main() must return an int:
int main(void)
We can make the array initialization easier to read with judicious use of whitespace:
static const int N = 5;
const int pixel_array[5][5] =
{
{1, 3, 6, 10, 15},
{2, 5, 9, 14, 19},
{4, 8, 13, 18, 22},
{7, 12, 17, 21, 24},
{11, 16, 20, 23, 25}
};
Instead of the if/...
2
As memory management is one of the question's tags, writing swap that way is NOT the way to go:
Harder to understand
Risks overflows
Is less efficient when looking at the compilation result: https://godbolt.org/z/iBlZ8n
The first thing to change if memory management is a concern is the allocation of the matrix. Allocate it as a continuous chunk of memory, ...
2
Instead of:
static_assert(SCHAR_MIN < -SCHAR_MAX && SHRT_MIN < -SHRT_MAX &&
INT_MIN < -INT_MAX && LONG_MIN < -LONG_MAX &&
LLONG_MIN < -LLONG_MAX && INTMAX_MIN < -INTMAX_MAX &&
INTPTR_MIN < -INTPTR_MAX && PTRDIFF_MIN < -PTRDIFF_MAX
, "Dinosuar: Non-2's complement."...
2
If you want people to use your library, then you want to think about how to make it easy for others to adopt it.
I would consider revamping the implementation into a Single-file library. Simply put, the more files, the more tedious it becomes to add someone else's code to your project. Which is why, single-file libraries are popular in the C/C++ world ...
Only top voted, non community-wiki answers of a minimum length are eligible
