25
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 ...
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.
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 <&...
7
Missing const
I see you sprinkled const almost everywhere. However, you actually missed the one spot where it actually matters most: input should be a const pointer:
_Bool fft(const complex_t* input, complex_t* output, const unsigned int size) {
...
}
Use the restrict keyword if possible
Since input and output are of the same type, they can alias. This ...
7
Answers to your questions
I've read that functions shouldn't normally take more than 2–3 parameters
I wouldn't worry about that too much. If you do have lots of parameters though, you should think about whether they are really necessary, and perhaps if there is a way to group them into a struct, and whether to pass that by value or by pointer. For example, ...
5
// Don't use these fields directly means that they do not belong to the public interface. Consider declaring
typedef struct ht ht;
in ht.h, and spelling it out in ht.c. Ditto for hti.
_ht_entry shall not be visible to the client at all; move its declaration to ht.c as well.
ht_set calls _ht_expand. In turn, _ht_expand calls ht_set. Even though it is (...
5
Major remark of the code as whole: for some reason you write the unreadable version of array access *(arr+i) all over the place. Don't do that! This makes your code look needlessly obscure and hard to read. Instead use the much more readable arr[i].
Your compare is just a naive implementation of strchr. It would be much more efficient to use strchr.
Don't ...
5
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 ...
4
Good header file
.... aside from the struct definitions that belong in the .c file.
Weak hash
OP's code only uses the least few bits of hash to form index and so this ht exercise entirely relies on the quality of _hash().
// Only uses a few bits of `hash`
size_t index = (size_t)(hash & (uint64_t)(table->_capacity - 1));
Consider a mod by prime for ...
4
Your is_operator function can be rewritten as
return token == '+' || token == '-' || token == '*' || token == '/' || token == '^';
Much cleaner, in my opinion.
The program has a memory leak. Your tokenize function allocates a struct token on the heap, but the object is never freed.
Better yet, instead of returning a struct token*, just return a struct ...
4
It looks like this code needs a couple of headers to be included before it will compile:
#include <stdarg.h>
#include <stdlib.h>
#include <string.h>
It also only works on platforms that have strdup(). It would be easy to make it more portable.
If s is a null pointer, then we have undefined behaviour, but we haven't indicated that we don't ...
4
Subtle functional bug
return *str1 - *str2; may return the wrong sign value when the char values are negative.
C string library functions specify:
For all functions in this subclause, each character shall be interpreted as if it had the type unsigned char (and therefore every possible object representation is valid and has a different value). C17 § 7.24.1 3
...
4
Your code doesn't compile cleanly with GCC:
move.c: In function 'memCopy':
move.c:16:15: warning: comparison of distinct pointer types lacks a cast
while (a2 < end)
^
Also 2 brief points:
Why reinvent the wheel? Most implementations of memcpy, memmove etc in the standard libraries will be correctly and efficiently implemented, in ...
4
The code is easy to read and to follow. There isn't a global in sight, the indentation is consistent, you use EXIT_SUCCESS and EXIT_FAILURE, and the control flow is simple. Therefore I have no comments about superficial aspects of style.
However, there is a bug in interpret: You are reading uninitialized memory. The code assumes that code will end in a null ...
4
#define PI 3.14... could use a few more digits!
sine should work for numbers greater than 2.0 * PI.
sine should work for negative numbers.
same for cosine.
if(temp > 2*PI) { temp -= 2*PI; } is ineffective for numbers greater than 4.0 * PI.
if(rem > 0){
// sine value for given argument isn't directly in the lut
if(index == (TABLE_SIZE-1)){
...
4
One remark is that you should get rid of all the needless branching and code repetition. It's bad for performance and code maintenance both.
Given an angle you should be able to:
Take it's absolute value.
Divide by PI/2.
Convert to unsigned integer, truncating decimals.
Then you'll either have an index from 0 to 3 or you started with an angle larger than 2*...
4
Lots of good info in the other answers, so here are a few minor points.
Use <stdbool.h>, if available.
Performance things.
Definitely avoid the mod operator. In this case, given your divisor is a power of 2, @harold's suggestion is great. In general, you can the same effect (every N'th item) with a counter:
if (--remaining > 0) { remaining ...
3
Unconditional masks
This:
if (byte & (1 << (CHAR_BIT - bit - 1)))
*value = (*value & ~1) + 1; /* 1 */
else
*value = *value & ~1; /* 0 */
is really just
*value &= ~1;
if (byte & (1 << (CHAR_BIT - bit - 1)))
*value |= 1;
Const arguments
void hide_file(char *filename, char *src_png_name, char *...
3
Usage: char *str = mutant_string ("file://", getenv ("HOME"), filename, NULL);
mutant_string("Hello", " ", "World", "!", NULL); contains a weakness.
NULL, a null pointer constant is not certainly a char *, a null pointer. It could be an int 0 for example and cause grief with va_arg(argv, char*) ...
3
Do not put the sentinel characters in every iteration. Set them before the loop, and after a successful reallocation.
Also, (*line)[*capacity - 2] = 0; is redundant. Indeed, if (*line)[*capacity - 1] remains 1 we don't care, and if it turns to 0 then (*line)[*capacity - 2] has been overwritten anyway.
goto is totally unwarranted. break works very well.
Do ...
3
The function does not handle end-of-file correctly. stdin is just a stream, and could be closed like any other stream. For example, type Ctrl-D. Or
echo -n a | ./a.out
a????????
Keep in mind that reading beyond end-of-file doesn't set errno.
There is no reason to special-case count == 0.
An IO error doesn't break the loop. If it happened, all the ...
3
In C, as in C++, object pointers can be freely converted to void*, so quite a few casts can be removed.
It's probably convenient to store data as a char*, since that's how we use it. We should still use void* as external interface, of course.
This is a dangerous anti-pattern:
p->data = realloc(p->data, VECTOR_RESERVE_SIZE);
If the ...
3
It looks good for a first program in C, good job.
An improvement would be to ask the user to insert a number which will be the maximum number to guess (instead of the default 100 value).
You also could change the try variable name to tries.
3
and welcome to C programming! Also, to Code Review.
Style
There are a couple of "style" issues with your code. The most obvious one is that your indentation is not consistent. I don't know if that's due to pasting it into the browser, or if it appears that way in your code. But computescore needs to be cleaned up.
Next is an issue of "...
3
The appeal of C is that it's simple, standard, and performant. Whereas your approach might (?) not impact performance all that much, it soundly destroys the first two concepts.
For reference, you're of course not the first to have tried crow-barring OOP into C. I somewhat recoil at the idea of dense, complex macro magic at all, much less to implement OOP. My ...
3
Formatting & Conventions
Always run your code through an autoformatter. On Linux, I use clang-format, and on Win10 I use VSCode's autoformatting intellisense features. I'm not sure of your exact setup, but formatting is big.
Next, variable names. The variable choose is wonderfully descriptive. I know immediately what it's for. The variable P_h on the ...
3
General Observations
Performance could be improved by reading all the contacts into memory at the start of the program and writing all the contacts back to a file at the end of the program. Searching a file for a duplicate is more time consuming then searching memory. This does however depend on the number of contacts being maintained.
Using a struct that ...
3
Input overrun
option is one character; so why do you write 3 here?
fgets( &option, 3, stdin );
Implicit string concatenation
printf( "\n\t\t***Contact management system***\n1. Add new contact\n2. Show all contacts\n3. Show specific contact\n*Press 'q' to exit*\n" );
is more legible as
printf(
"\n"
"\t\t***...
Only top voted, non community-wiki answers of a minimum length are eligible
