Hot answers tagged

26

Your implementation is going to be slow, and the excuse "I need it to take a fixed amount of time" does not justify this. Using plain tables smells like cargo culting as well. So I'm not tackling what you did wrong in your code, but what you did wrong in even thinking about your implementation. First, google how to implement cosine on a micro ...


23

You should keep your game loop as simple as possible with only an update and draw method and specifically in that order. To ensure your game loop is iterating at the same speed everytime, you should calculate the duration of your update and draw methods and then subtract that from how long each iteration should last. i.e.: MD = 10ms // max duration for an ...


19

The interface is problematic. What output does this code produce? #include <stdio.h> int main(void) { const char *s1 = uint32_to_binarystr(0); const char *s2 = uint32_to_binarystr(1000000); printf("%s\n%s\n", s1, s2); } You might expect it to output 00000000000000000000000000000000 followed by 00000000000011110100001001000000, ...


19

Calling strcat in a loop is inefficient The function strcat works the following way: It must first search the entire the destination string until it finds the terminating null character at the end. It then appends the source string. Therefore, calling the function strcat in a loop in order to append one character at a time is highly inefficient. This is ...


19

Neat and well formatted Design Rather than "then after every string, the 'delim' string is concatenated." (that sounds like n delimiters), I would expect between every string. between, or the like, matches code (n - 1 delimiters). Design: num_args == 0 Consider allowing this corner case. // if (num_args <= 0) return NULL; if (num_args < 0) ...


16

Your execution time is \$O(len(str)*len(remove))\$ which is slow. Since you're restricting yourself to ASCII you can create a LUT like so bool remove_lut[256]= {false}; while(*remove) remove_lut[*((unsigned char*)remove++)] = true; Then remove the entire section where you scan the input for all charges in remove. And replace found with remove_lut[*((...


16

Versatility The pointer version is less versatile. If the two values come from expressions like in min_value(x + 13, 2 * y), you can't use the pointer version, as neither x + 13 nor 2 * y exist as pointable memory locations, only as temporary values. Surprises The pointer version might give surprising results in the long run. If now first is lower than ...


15

void main is a really bad habit. It must be int main. Always check what scanf returns. For example, try to enter a non-numeric input, and see your program entering the infinite loop. Avoid conio.h (and hence getch). It is very non-portable.


12

#define growCapacity(capacity) \ (capacity < 8) ? 8 : capacity*2 I don't think this should be in the header, as it's not useful to the user of our vector. Consider moving it to the implementation file. int count; int capacity; int* Array; count and capacity would be better as size_t values - especially as we use size_t arithmetic with them. ...


11

Avoid code duplication Your code is more verbose than necessary, because you are repeating a lot of things unnecessarily, or write things in a more complex way than necessary. For example: (index + (index + length)) / 2 This looks really weird, and is actually equivalent to: index + length / 2 Which makes much more sense. Even with that change, that ...


11

#define EXIT_SUCCESS 0 You are not permitted to define standard library names yourself. So just include <stdlib.h>, or omit the return from main() (remember, C will provide a success return if you don't provide one). void fizzbuzz(int n) { We could give that internal linkage: static void fizzbuzz(int n) { char result[9] = ""; // strlen(&...


10

More specific types are available for an eight bit number, such as uint8_t static keyword is dangerous (when used as an output buffer) and results in weird behaviour if your method is called twice The arithmetic can be replaced with a more simple & (bitwise AND) void to_binary(uint8_t x, char *output) { *output++ = '0'; *output++ = 'b'; for (...


10

About the manpage The manpage looks very nice! There's a few small improvements that could be made: Spelling: "decrescend" should be "decreasing". Some conflicting statements are made, like "Each event must begin with an optional status." Consider rewriting this to avoid any confusion. Perhaps also start with a single line ...


10

Some beautifying I think first < second ? first : second is a tiny bit more readable. First comes first. Double problems Floating points are tricky. What will happen if one of doubles will be NaN? Of course, you can always say that the function assumes both values are valid, but you should consider this too. Pointer problems Pointers are also tricky. ...


10

Overall clear and readable. Some remarks: atoi is one of those standard library functions that should never be used. It is 100% equivalent to strtol family of functions, except that the latter have error handling and support for other number bases than decimal. Since you are using this function on user input which has not been sanitized, you should not use ...


9

We're missing an include of <stdint.h> in the header, and <stdbool.h> in the implementation. Some parts of the code are hard to read. In particular, I think this comment should be wrapped at a more conventional line length: /* * Description: This module is used to accumulate data when the network is down or unable to transmit data due to poor ...


9

Enums In the code s is used to indicate the movement direction. From the code it's not easy to tell what s or s == 2 means. An enum would improve readability and maintainability of the code. typedef enum { Up, Down, Left, Right } Direction; Which would lead to something like this: Direction direction; if (kbhit()) { switch (getch()) { ...


9

Efficiency Your code is already more efficient than you think. It has time complexity \$\mathcal{O}(n)\$, and thanks to tail recursion, it also has space complexity \$\mathcal{O}(1)\$. Avoid forward declarations I recommend that you avoid the need to forward-declare arrange() and swap() by reversing the order in which the functions appear in your source file....


9

Non-exported functions Declare get_valid_input and flush as static since you're in a single translation unit. Foot-cannon one-liners while ((printf(PROMPT), (a = scanf("%lf", &inputNum)) != 1) takes the common C antipattern of in-condition assignment even further: it's an entire code-block in a condition. Please consider rewriting this to ...


9

Your code has a possible bug, depending on your CPU/compiler. You are treating UINT32_MAX (0xFFFFFFFF) as a special case, however, you could run into problems with any value greater that 0x80000000. For example, if we let n be 0x80000001, then ... int expb2 = 0; ... while (n != 0) { while (1 << expb2 <= n) { expb2+...


9

it seems to be optimized quite well by the compiler, as it uses a lot of vectorized instructions Well, kind of. GCC didn't use vectorization at all, and Clang used some of it but in a very strange way. Let's look at what it's doing. Basically I'm reviewing what Clang did, not so much your code.. but that then informs you about your code by proxy. I'll go ...


9

Counting the digits doubles the amount of work, and the division/modulo is indeed very hard work for the CPU. You know the maximum size of the resulting string, because it would be the largest or most negative integer you can be passed. So use a local array as a buffer, and then copy the result to allocated memory. You have a separate function to reverse ...


9

Shift the constants not the variable if (column_sum & 1) { even_column_code ^= 7; odd_column_code ^= 0;} column_sum >>= 1; if (column_sum & 1) { even_column_code ^= 6; odd_column_code ^= 1;} column_sum >>= 1; if (column_sum & 1) { even_column_code ^= 5; odd_column_code ^= 2;} column_sum >>= 1; if (...


9

You can use memmove() function to move overlapping areas of memory and memset() to clear the moved items. LeftShift can be implemented like: #include <string.h> void LeftShift(struct Array *arr, int n) //n is the number of shifts { memmove(&arr->A[0], &arr->A[n], (arr->size-n)*sizeof(arr->A[0])); memset(&arr->A[...


9

You know, C and C++ are different languages. You used both tags, and your code is confused as to which it wants to be. It appears to use C syntax, library calls, and ways of doing things; but it includes a C++ header. It doesn't seem to use that header though. Maybe this is the current state of the file after you have done some experimentation and ...


9

Reserved identifiers The include-guard _STRCAT_NEW_H_ is one of the names reserved for the implementation, as it begins with underscore and an upper-case letter. As a general guide, never begin names with underscore. The function name beginning with str is reserved for standard library extension, so choose a different name there, too. Interface Consider ...


9

Use Helper Functions Although my coding style hasn’t caught on, I think ternary ? : expressions lend themselves very well to algorithms like this. const DateTime* DateTime_min( const DateTime* const a, const DateTime* const b ) { return (( a->year > b->year ) ? b : ( b->year > a->year ...


8

Design: Code should explain cases Double space: "abc xyz" --> Is that to be 2 or 3 tokens? Leading space: " abc xyz" --> Is that to be 2 or 3 tokens? Trailing space: "abc xyz " --> Is that to be 2 or 3 tokens? Design : Generalization split() only considers ' '. Maybe all white-spaces, '\t', '\n', ...? Or pass into ...


8

Bit-reversal permutation bug Keeping both a normal counter and the "reversed counter" is the right idea, but this implementation isn't quite right. For example, it might result in a sequence such as 0, 4, 2, 6, 1, 3, 5, 7 while the correct one is 0, 4, 2, 6, 1, 5, 3, 7. A B 000 000 100 100 010 010 110 110 001 001 011 101 <<< 101 011 <&...


8

It's not terrible as it stands, but I think there are some ways it might be improved. Include all needed files The function needs several #include files that are not listed. Specifically, it needs these: #include <stddef.h> #include <stdio.h> #include <limits.h> It's important to list those, especially for a beginner. Be careful with ...


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