New answers tagged

1

Comment about your naming conventions. Statements such as current = original show that you conflate the array and the pointer view of things. Clearly original refers to a string, whereas current seems to be "the current location in the string". (I would be almost tempted to write current = &(original[0]) just to show what's happening.) However, the name ...


1

As mentioned in another answer, the best improvements to the code would be a function that is the equivelent of the C library function strstr(char *hay_stack, char *needle) and a function that replaces the string. This would simplify the main() function. One of the basic ways of writing a program is to keep breaking the problem down until the result is small ...


3

It would be interesting to eliminate the loops; I suspect the optimizer will deal with the hard coded indexes and optimize the code. character0[0]=card[0]; character1[0]=card[2]; character2[0]=card[4]; character3[0]=card[6]; character4[0]=card[8]; character5[0]=card[10]; character0[0]=card[1]; character1[0]=card[3]; character2[0]=card[5]; character3[0]=...


0

As x ^ x == 0 on can do a homogeneous: return nx[0] ^ nx[1] ^ nx[2] ^ nx[3] ^ nx[4] ^ nx[5] == 0; A char, a hex digit represent a nibble, 4 bits. As XORing does not influence other positions (like ADDing), one can do the checksum separately on the two nibbles: int hex(char ch) { // For ASCII return ch <= '9' ? (int) ch - '0' : (ch & 0xF) + ...


4

Aim It's not 100% clear in your question, but what I am assuming is that you would like some feedback on: How to utilize C++ for this problem domain without using the STL (ie namespace std::) How to improve / re-structure the code to make it "better and shorter". Just a general point. Obviously both C and C++ have standard functions / algorithms for ...


9

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


5

The most obvious issue is that you create excessive double loops for the same type of array initialisations. Convert similar loops into one Instead of creating 6 separate double loops for each character array, set each array in the same double loop: ... for(int i=0;i<2;i++) { for(a=0;a<2;a++) { character0[a]=card[i]; character1[a]...


0

For future reference: Debug code should be embedded in #ifdef DEBUG #endif Having live debug code in the program means the code is not ready for code review. Having unused functions such as char *input(const char *q) means the code is not ready for code review. Performance Shells provide the user interface for the operating system, they need to be ...


3

1201ProgramAlarm rightly comments: Is __xObject_Private_data_t defined in the same header as xObject_t? If so, the application writer can do the same thing you are to get at the data. In other words, your chosen approach is no solution at all. Besides, your chosen approach is buggy! A C data type consists of more than its sizeof. You also have to ...


0

A self answer per: If you want to show everyone how you improved your code, but don't want to ask another question, then post an answer to your own question. @Mat identified undefined behavior of using relational operators with unrelated pointers: if (format >= GP_format[0] && format < GP_format[GP_format_N]) A fix: 1) Make each GP_format[] ...


2

Overall good job, the code checks malloc() and sometimes checks scanf() for errors, generally braces wrap logic in if statements and loops. The functions generally adhere to the Single Responsibility Principle. It's not clear why the function createNewState() is unused since this would greatly simplify the main() function. There should be a function ...


3

About your notes There are no error messages to handle ill-formed input. We were required by those who gave us the assignment to focus on solving the problem, rather than making it become an input-checking exercise. So I'll pretty much assume the input is well-formatted throughout the program It is a good excercise for yourself to add input checking. ...


1

On my computer while the program runs and gives answers it still terminates with 0xC0000005 which means the memory problem is still there but it no longer stops the program. The type of recursion you are using, where 2 functions (int succ_value_smaller(struct list * ptr, int * V, int * count, int k) and int complete_list_to_array(struct list * ptr, int * V, ...


3

gcdn[] is unused. if n==0, you access a non-existent array element, and even if it isn't, you do the GCD with a[0] twice. gcd() is called before it is declared, which is bad style and can mess up optimization. you might think about whether the M or arr[] elements are likely to be larger. The order you pass values to gcd() can reduce by one loop if you ...


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


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


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

Before getting into what can be improved, the good points are that there are no global variables and there are many small functions that perform small operations which makes the code easier to read, write, maintain and debug. Prefer calloc Over malloc for Arrays There are 3 major allocation function in the C programming language, they are void *malloc(...


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


1

unsigned vs, unsigned long Code mixes use of unsigned and unsigned long as if they are of the 32-bit size. In 2020, 16-bit unsigned is commonly found in embedded processors and 64-bit unsigned long in 64-bit processors. Do not assume unsigned, unsigned long size/range other than they are at least 16, 32 bits. #define UINT_MAXIMUM 4294967294 // UINT_MAX ...


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


1

It's been a long time since I've heard the terms BCD or Binary Coded Decimal. Keep in mind that BCD was primarily a hardware encoding of integers provided by one particular manufacturer of main frame computers. There were computer performance issues when using BCD. The best way to implement a BCD number would be an array of 4 bit integer values. One good ...


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


2

What should the output be if I enter 123456789012345678901234567890? That's producing an integer overflow right now.


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


2

There's a subtle issue with this line in the get_random_card function: srand(time(NULL)); You're seeding the random number generator with the current time, each time get_random_card is called, which means multiple calls within the same second will have the same sequence of random numbers. In this specific case, it doesn't affect anything (other than a ...


1

Spelling "Suite" is not the same as "Suit". A suit would be Hearts, Clubs, Spades and Diamonds. Logic Checking Why is MAXCARDS 25? In Blackjack, the most cards a player is allowed is 5. Even then, the target is 21; with a standard deck of cards, that would be 4 Twos (8), 3 Threes (9) and 4 Aces (4). If you are planning on playing with more than 1 deck, ...


1

Stack efficiency Is the SceneFSM__Run method stack efficient? There is no recursion, so stack usage is bounded. The only way to make it more efficient is to have less on the stack in each function, but since you don't have much to begin with I would not worry about this. How to switch scenes In order to switch to another scene, the currently running ...


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


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


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


1

First off, the second loop should be a for loop: for (i = 0; i < len; ++i) /* what i think is unnecessary */ { s[i] = rev[i]; /* storing back to initial array */ } But to eliminate that, try: while(i < l) { char tmp = s[i]; /* swapping */ s[i] = s[l]; /* swapping */ s[l] = tmp; /* swapping */ ++i; --l; } If there is a true ...


1

Standard C requires that main() return int: int main(void) There's no need (for 20 years now) to write all local declarations before the code. Introduce variables as you need them, so they can be initialised immediately: { char line[MAXLEN]; int i = 0; There's no need for the extra storage rev in reverse(). We can reverse the string in place, ...


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


1

Neither C nor C++ are the friendliest languages to get acquainted with programming, and character I/O is not their forte either. Besides, these raw C arrays/pointers are both an eyesore and a headache. int temp, letter = 0; I wouldn't mix uninitialized and initialized variables. For an educational code, each variable should be on a separate line with ...


-2

Revision #2 #include <stdio.h> #include <stdlib.h> int main() { //A string is an array of characters static const char alphabet[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; int number = 0; //while(number is not between 1 to 26){get number/input} while( (number < 1) || (sizeof(alphabet) < number) ){ const int c = scanf("...


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


0

A cardinal rule of good programming is to process return values appropriately. Specifically, getchar() returns an int, not a success true/false boolean. Not processing the return value properly (here: while(!getchar())) leads to various undesirable effects, including the problem with the endless loop when fed an empty file. Additionally, as numerous others ...


2

Robust input is difficult to get right in C! The input loop is broken when we reach end of file: it will loop indefinitely. What we want to do is to give up completely when we get EOF back from scanf(): int items = scanf("%d", &letter); if (items == EOF) { fputs("Failed to read input\n", stderr); return EXIT_FAILURE; } if (items != ...


-2

This is the revised version #include <stdio.h> int main() { const char alphabet[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; int number = 0; while( !(1 <= number && number < sizeof(alphabet)) ){ if(!scanf("%d", &number)){while(!getchar());} } printf("The number %i corresponds to the letter '%c'\n", number, ...


2

Never forget the trailing nul. sizeof(alphabet) is 27, not 26. Don't use == false. It is too close to == true, which almost never works, especially in C. Use prefix operator ! instead. Then, as others suggest, consider reducing with de Morgan's laws, or maybe not. (EOF & '\n' & '\r') evaluates to 8 (usually). This is not a set operation. ...


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


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


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


1

// Not following Swift naming conventions, should be bytesPerPixel let BytesPerPixel = 4 // Better to make function throwable and throw specific error. guard let srcProv = srcImg.dataProvider else { fatalError("dataProvider is nil") } guard let srcProvData = srcProv.data else { fatalError("dataProvider.data is nil") } For unwrap of srcImg.colorSpace! is ...


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


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


Top 50 recent answers are included