Hot answers tagged

56

Each blood factor can be present or not present. The blood types can be bit-encoded using one bit for the A factor, one bit for the B factor, and one bit for the Rh factor. enum { A=1, B=2, Rh=4 }; You would parse the blood type "AB+" as A + B + Rh == 7, and "O-" as 0 because it does not contain any A factor, B factor, or Rh factor. Survival requires the ...


45

This is a pretty reasonable start on a simple interpreter. Edward's suggestions are all good; a few additional suggestions: interpret("+++++++++++++[->.... Please break up that long line. C allows you to break up literal strings "like " "this." void goToLoopEnd(char** ip) { ... void goToLoopStart(char** ip) { ... If you wrote these instead as char *...


36

I see some things that I think could help you improve your code. Decompose your program into functions All of the logic here is in main in one rather long and dense chunk of code. It would be better to decompose this into separate functions. Check return values for errors The call to scanf can fail. You must check the return values to make sure they ...


28

Typo lenght is spelled length. Magic numbers What does 95 signify? You'll want to put this in a named #define or a const. Allocation failure After calling malloc, always check that you've been given a non-null pointer. Allocation failure does happen in real life. Indentation You'll want to run this through an autoformatter, because your if block has ...


28

Your code is way too complicated. If any of A, B, or + is present in the donor, it must also be present in the recipient. That's all you need. bool is_compatible(const char* donor, const char* recipient) { if(strstr(donor, "A") != NULL && strstr(recipient, "A") == NULL) return false; if(strstr(donor, "B") != NULL && strstr(...


26

Here are some things that may help you improve your program. In all, it seems to be nice, straightforward code that does what it needs to do. Good start! Use only required #includes The code has #include <stdbool.h> but doesn't use booleans. It also appears that nothing from <stdlib.h> is used either. Only include files that are actually ...


22

Efficient file I/O By default, files opened with fopen() are buffered, meaning that not every call to fread() or fwrite() will result in a system call. Instead, the C library has an internal buffer and will try to read and write larger chunks at a time. However, you are still paying for the overhead of a regular function call each time you call fread() and ...


21

typedef enum { EQUAL = 0, LHS_NEWER, RHS_NEWER } ECOMPARISON; A common convention for three-way comparisons is that of strcmp() - return any negative value if the first argument is less than the second, zero if equal, and positive if greater. If we take that approach, and if we can ensure that the version values are small enough to avoid ...


21

log is already declared in <math.h>. You don't need to declare it yourself. In fact, it could be harmful. As stated in another answer, do not use floating point math. In fact, you don't need to know the exact position of the leftmost bit. For your purposes, the value of codepoint is enough. For example, bitPos <= 7 is equivalent to codepoint < (...


20

Use a 2D array instead of 6 1D arrays (vs. character0, character1, ...) Use a 1D array for n instead of (n0, n1, ...) Add EOS termination to the string that gets passed to strtol Here's a refactored version: bool checksum(char card[]) { char chars[6][3]; long nx[6]; for (int col = 0; col < 6; ++col) { int lo = col << 1; ...


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


18

This answer uses pointer-casting for type-punning just to save space. In practice keep using your union (safe in ISO C99, and in C++ as a GNU and MSVC extension) or memcpy (safe in C and C++). This pointer-casting is only safe in MSVC, or GNU-compatible compilers with -fno-strict-aliasing Initial approximation Packed bit fields are not only unnecessary ...


18

Welcome to Code Review, we wish you the best. General Observations Congratulations on getting this to work, one function that is 356 lines long and almost 17K of text is a bit large and very hard to code and debug. Complexity The function main() is too complex and should be broken into functions. A general rule of thumb in all programming is that a function ...


16

It's a good idea for worker.c to include "worker.h", preferably as the very first line. That ensures that the declarations are consistent and that the header has no missing includes of its own. (So, in task.c move the task.h include to the top spot, and similarly with utils.h in utils.c.) I recommend adding include-guards on all your header files. I know ...


15

Following @Martin R's comment, I'll make my comment above a solution: printf already supports what you're trying to achieve with your if-then-else jungle: #include <stdio.h> int main() { int T; scanf("%d", &T); for (int i = 1; i <= T; i++) { int N; scanf("%d", &N); int hours = N / 60; int ...


15

/* Asks the user for string input. * Returns a pointer to the string entered by the user. * The pointer must be freed. */ Slightly misleading in that this function doesn't ask for input. (As written it is not responsible for printing the prompt.) Perhaps also should clarify the intended behavior: If there is input that is not terminated by a newline, ...


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


14

The CompareVersions() function in this answer uses subtraction for comparison. This is considered to be bad practice - it leads to bugs and potential security holes. (Yes, the post does say "if we can ensure that the version values are small enough to avoid integer overflow", but that pretty much requires the caller of this function to know the result ...


13

Minor stuff ... Allocate to the object, not type The below is easier to maintain. // char **aux = malloc(sizeof(char *)) char **aux = malloc(sizeof *aux) // aux[0] = malloc(bufsize * sizeof(char)); aux[0] = malloc(sizeof *(aux[0]) * bufsize); Avoid Exploit Below code is undefined behavior is the first character of user input is the null character. It ...


13

There is a bug in this line: password[i] = symbols[rand() % 26]; symbols is only 21 characters long, so this line triggers undefined behavior when rand() % 26 is greater than 21 (and when rand() % 26 is exactly 21, it puts a null byte in the password). You meant for this 26 to go with letter instead, and for the 21 to go with symbols. This would be less ...


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


13

Make sure Doxygen picks up comments after struct members It's good to add a comment after each member in a struct declaration to describe what that member does. To make Doxygen pick up the member documentation as well, use //!< or ///< instead of // to start the comments with. Use "filename" or "path", not "file" describe filenames You will be ...


12

These declarations are not prototypes: void getDataFromRapidSrcFile(); int getTotalPositions(); These declare functions that can be called with any number of arguments. It appears that they should take no arguments; we indicate that like this: void getDataFromRapidSrcFile(void); int getTotalPositions(void); It's a good idea to make the same change ...


12

Naming It's unconventional to name a type with all-uppercase - we normally reserve those names for preprocessor macros, to warn readers that they need treating with care. Avoid such names for ordinary identifiers. Avoid using identifiers that begin with an underscore - in many situations, those names are reserved for use by the implementation, which could ...


12

Prof's transcribed code has weaknesses #include<stdio.h> int main(){ int n; while(scanf("%d",&n)==0 || n <2 || n >20){ while ( getchar () != ā€™\nā€™); printf( "Wrong. Try again\n"); } printf ( "%n\n",n); return 0; } Infinity loop When input is closed after a scanf("%d",&n) == 0, getchar() can eventual ...


11

Whilst your code works, there are a number of simplifications that you might try. As Reinderien says, get rid of "magic" numbers Having done that, declare a single string containing all 95 characters with the special ones last. This does away with all the strcat code. It's good practice to declare has_special_characters as type bool. You will have to ...


11

Avoid global variables - there's no need for t and v to exist outside main(). Always check the return value of scanf() before using the written values. Don't assume that CHAR_BIT is 8, or that sizeof (short) is 2. Neither of those is portable. Don't assume a particular ordering of bit fields within a struct - that's entirely compiler-dependent. Portable ...


11

I'm not particularly familiar with C, but I have a few observations that may be helpful. I like the attention to zeroing out your digest before you start. Using uninitialised memory is the sort of bug that can go unnoticed for a while, and it's good that you've caught it. At the same time, I'm slightly alarmed that you're doing everything with raw loops, ...


11

Don't strtok + atoi. Use strtol, which (a) doesn't need a mutable input, (b) has much better error handling and reporting, and (c) eliminates the need for independent validation. An example of use would be char * end; pVer->major = strtol(str, &end, 0); if (*end != '.') { // major is not a number. return suitable_failure; } str = end + 1; ...


11

This program reads 4 byte codepoints (in BIG ENDIAN) from a file strictly called "input.data" and creates another file called "ENCODED.data" with the relative encoding in UTF8. Needless to say, that's a weird way of storing code points. I know UTF-16, but UTF-32BE (just the code point in big endian form) is not widely used, although Python seems to use ...


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