New answers tagged

5

Here are some things that may help you improve your program. Use consistent formatting The code as posted has inconsistent indentation which makes it hard to read and understand. Pick a style and apply it consistently. Provide complete code to reviewers This is not so much a change to the code as a change in how you present it to other people. Without the ...


2

Here are some things that may help you improve your code. Use a newer reference K&R second edition is quite old and refers to the 1989 version of the C language. If you want to learn the C language, a more recent source would likely be more useful. The current version of the standard is C18. Be careful with signed vs. unsigned You are correctly using ...


5

General Observations and Comments You have obviously put in a lot of effort into this question and deserve an answer. I have mostly programmed in Python, C++ and Java so far, and I recently picked up K&R (second edition) to learn some C. I have only gone through the first chapter so far, which is "A Tutorial Introduction". K&R was the ...


7

Use Common Definitions Rather Than Hard Coded Values I agree with @pm100 about NULL, it is much more common to use NULL rather than 0x00. Very early C++ compilers also used NULL rather than nullptr. Since stdlib.h is already included, the exit constants EXIT_SUCCESS and EXIT_FAILURE are availble, this would make the code more readable and maintainable. Most ...


3

my 2 cents worth v nice clean code. I would have called the generated variable N not self_N. That looks peculiar , plus all the other generated names are N## something, having something##N is also odd. In the macro call I said I wanted it called 'readBuff' so call it that. the use of 0x00 for null is certainly correct buts its the first time I have ever seen ...


1

for(n = 0; cumsum <= u_rand; n++) cumsum += (double) distribution[n]/(double) norm; is not clear to me that distribution[n] will always use a n in the [0... LATTICE_SIZE-1] range. I suspect, depending on the roundings and sequencing of adding floating point numbers, code may iterate too far and attempt distribution[LATTICE_SIZE] which is bad. Best not ...


-1

This will definitely work.... By using only getchar() and putchar(), it can be made easily. #include<stdio.h> int main(){ char ch; printf("Enter string:"); while((ch=getchar())!=EOF){ if(ch==' ') { while((ch=getchar())==' '); putchar(' '); } ...


4

The output is not uniformly distributed The resulting characters in the output string are not uniformly distributed. In fact, some characters never occur in the output. After shifting a random byte right by two, if the result is larger than or equal to 32, you always clear the second least significant bit. So all values up to 32 are possible, but then it is ...


2

When compiling, always enable the warnings, then fix those warnings. A run of the code through the gcc compiler results in: gcc -ggdb3 -Wall -Wextra -Wconversion -pedantic -std=gnu11 -c "untitled2.c" -o "untitled2.o" untitled2.c: In function ‘calculate_avg_density’: untitled2.c:56:31: warning: conversion to ‘double’ from ‘long long ...


1

(s)size_t Use size_t (It is in <stddef.h> in standard C) or ssize_t (<sys/types.h> in POSIX systems) for raw sizes in bytes. ptrdiff_t Use ptrdiff_t (<stddef.h>) for element counts such as List.size or array indices such as [i]. Data type Don't restrict the data to int. Use void *arr instead so that the user can fill the array with any data....


4

Naming things Naming things is one of the hard things in computer science. You should make sure names of functions and variables are clear and concise. In general, use nouns for variable names and verbs for function names. Looking at your choices of names in more detail: LATTICE is not a lattice, it is the size of the lattice. So call it LATTICE_SIZE. ...


2

Just a small idea: Handle len==0 Just add a little code to gracefully cope with len==0 rather than UB. char *format(const char* format, char *buf, size_t len, ...) { ... // 0 -1 is SIZE_MAX! // for (; *format && i < (len-1); ++format) { for (; *format && i + 1 < len); ++format) { ... // buf[i] = '\0'; ...


2

Return the number of bytes written to buf You will notice that functions like sprintf() and strftime() don't return a pointer, but rather an integer that says something about the number of bytes that (would) have been written to the output buffer. This is much more useful than just copying the pointer to buf, which doesn't give the caller any new information....


2

I am trying to write the fastest possible string interpretation algorithm possible. As "fastest" depends on many factors. Not only post a function to test, but the data set and test harness used to assess speed. Otherwise we are left making general remarks. Bug Word|32 is certainly wrong to fold the character case when the input does not begin ...


3

How can I improve my function, to not be as redundant while checking if Len is equal to 1? Make the null character part of the check. Len not needed anywhere. // if (Len == 1 || (Len == 5 && !memcmp(Str+1, (const char[]){'a', 'l', 's', 'e'}, 4))) { if (StrIn[1]==0 || !memcmp(Str+1, (const char[]){'a', 'l', 's', 'e', 0}, 5))) { // or if (StrIn[1]==0 ...


1

Bug: is_numeric("") returns true. Bug: is...(negative_values) is UB. (Aside from is...(EOF)). Possible when *str < 0. Bugs: as reported by user3629249 Consider allowing hex such as "0xAbC". Standard library string functions operate as if char is unsigned, even if char is signed. Recommend to do the same here. What do you think ...


3

regarding: if (!isdigit(*str) && !isspace(*str)) the isdigit() handles 0...9, so catches if any of the passed in char array is not numeric. The && !isspace(*str)) has nothing to do with numeric values


4

Your Concerns The use of pow(i, 2) is unnecessary; you should simply use i*i. The goto statement is also unnecessary. You could wrap the code in while(true) { ... }. Other Concerns The cast (int) i is also unnecessary, as i is already an integer. Perhaps you meant to cast to a double? Computing sqrt(n) in the for loop termination condition is inefficient; ...


2

float vs. double Little reason to use float here, suggest double instead. Save float for selective space/speed issues - which are not present here. If code uses float variables, use float functions like sinf(), log10f(), powf(), ... than sin(), log10(), pow(). Printing floating point Rather than printf("%f", ans), print using "%g" or &...


3

Pi There are some dissenting opinions on this, but: most compilers offer an M_PI if they are configured to do so. I prefer to do that rather than defining my own. Implicit return types Leaving these return types implicit: load(); //Function Prototype Exit_0(); //Function Prototype main() { is not great. main should be int main. Exit_0 should be void. load ...


7

is this code efficient ? No. Rather than using the unnecessary powe(), lent(), simply scale by 10 each operation; and use - '0'. . char sn[20]; //scanf("%s",sn); if (scanf("%19s", sn) == 1) { int z = 0; for (int i = 0; sn[i] >= '0' && sn[i] <= '9'; i++) { z = z*10 + (sn[i] - '0'); } printf("%d",z); }...


4

regarding: scanf("%s",sn); To avoid any possible buffer overflow, should include a MAX characters modifier that is 1 less than the length of the input buffer ( 1 less because the %s input format conversion specifier always appends a NUL byte to the input. Should check the returned value to assure the operation was successful. Note: the scanf() ...


6

Welcome to code review. Now my question is, is this code efficient ? or did I use the "long way". You used the long way. The solution you provide is not portable to systems that don't use ASCII. Use '0' to '9' instead. Rather than iterating through each of the ASCII characters do a range check: if (a >= '0' && a <= '9') { ...


2

Array sizes uint32_t may be too small or needlessly large to index arrays. Use size_t for array indexing and sizing. //int huffman_encode(uint8_t * input, uint8_t ** output, uint32_t decompressed_length); int huffman_encode(uint8_t * input, uint8_t ** output, size_t decompressed_length); Namespace spattered Rather than huffman.h include defines/functions ...


3

Pure Functions vs. Global Variables You might want to consider making the function pure. That is to not use global variables. Pass those variables as additional arguments. Although global variables an nonpure functions have their place in some use cases and in C in particular, like embeded systems, small self contained programs/modules, etc. But I would say ...


11

The SIMD code has type errors. The problem is currently a bunch of floats are read, assigned to a SIMDi anyway, added as floats (and remember, these were integers, reinterpreted as floats), assigned to a SIMDi again, and then stored as floats (into an array of integers). The type warnings are not just noise, it's also actually wrong: integers are being ...


8

There is no need to implement stringEqual yourself. Use strcmp(a, b) == 0 instead. For all variables that contain memory sizes, you should use size_t instead of int. In C, the function strlen is terribly slow since it has to scan the entire string for the terminating '\0'. Never use strlen in a loop for the same string. Bug: as soon as your word list ...


9

The only obvious thing I see is that your realloc machinery is too complicated given the task at hand. Your entire word file is only 530 KB. You could easily iterate over the entire file to count newlines and get the exact array length you need, and this would be done quite quickly. If alignment of the resulting strings is crucial, you'd want to keep your ...


0

First off, you could use constants for the command strings: char COMMAND_SET[] = "SET"; char COMMAND_GET[] = "GET"; The processing of the two commands can be slightly generalized: if (arr[1] == NULL) { return ERR_NO_KEY; } // Validate key length if (strlen(arr[1]) > MAX_KEY_LENGTH) { return ERR_KEY_LENGTH; } c->key = arr[1]; ...


3

A bug This version of the program uses limited-length codes, which is good. Decoding looks good. However, limited-length codes create a new edge case: what if the tree is deeper than the length limit? There are various solutions, but as far as I can tell, none of them are used in this program - a length that exceeds MAX_CODE_LEN is generated and things go ...


6

Magic buffer sizes Consider making a const or #define for 256 and 65536. Const inputs uint8_t * input should be const uint8_t *input since you don't (and shouldn't) change it. Loop combination This: for(size_t i = 0; i < decompressed_length; i++) freq[input[i]]++; for(uint16_t i = 0; i < 256; i++) if(freq[i]) encoded_bytes++; does ...


7

Returning by reference ttt_init makes an instance of your TTT_BoardGame but that instance has to be copied when returning it. This cannot be done efficiently. Your struct is so trivially small that no performance impact can be noticeable, but since you're expressly doing this for learning, it's worth pursuing the idiomatic method instead: Either accept a ...


12

Return the condition When we test for equality, a == b is either 1 if they're equal or 0 if they're not (see C11 §6.5.8 Relational operators). Therefore we can simply return the expression instead of an if-else construct: bool fifo_is_full(struct fifo * fifo) { return fifo->elements == fifo->size; } bool fifo_is_empty(struct fifo * fifo) { ...


3

I'm trying to rewrite it from scratch. How does this look? Mis-matched function signature OP code uses void *MemContainsMem(const void *StrStart, register unsigned long StrLen, const char *Substr, register const unsigned char SubstrLen); The referenced code uses void *memmem(const void *haystack, size_t haystacklen, const void *needle, size_t ...


0

When I run the game, it allows me to select a side. I selected 'x'. Then the display changed to add the 'o' side, then the display commenced to flash with no (discernible) way for me/computer to make a move. suggest the system( "clear" ); only be called after a side inputs a move.


Top 50 recent answers are included