Hey everyone! I'm a newbie to C programming and I've written some code that I would love to get feedback on. If you've got a moment, could you please take a look at my code below and share any insights or suggestions? Here's what I have so far:
```c
#include
#include
#include
#include
#define __GNU__IS__NOT__UNIX__
#define g_ARRAY_SZ 24
int main(void) {
char cl_array[g_ARRAY_SZ] = {0}; // Create buffer
bool bl_stat_flag = false;
printf("Insert valuen");
scanf("%s", cl_array);
if (g_ARRAY_SZ - 1 <= strlen(cl_array)) { // Check value length
printf("Buffer over flown");
return -1;
}
for (int i = 0; i < g_ARRAY_SZ; ++i) {
if (0x00 == cl_array[i]) { // Check null value
bl_stat_flag = true;
if (0x00 == cl_array[0]) { // Check first null value
printf("First value is nulln");
return -1;
}
break;
}
}
for (int i = 0; i < g_ARRAY_SZ; ++i) { // Find upper or lower and exchange char
if ((char)65 = cl_array[i]) {
cl_array[i] = tolower(cl_array[i]);
continue;
}
cl_array[i] = toupper(cl_array[i]);
}
printf("-> %sn", cl_array);
return 0;
}
```
Thanks so much!
2 Answers
Hey! First off, your use of `#define __GNU__IS__NOT__UNIX__` doesn't seem to serve any purpose here, so you might want to consider removing it. Also, using `scanf` directly like that can lead to undefined behavior if the input string is too long. It’s much safer to use `fgets`, and always check the return value. By the way, returning negative values from `main()` is a bit unusual; you usually return `0` for success. For character comparisons, it’s clearer to just use 'A' instead of (char)65.
Main returning negative numbers as an error indication isn't so uncommon. Some people do it for signaling errors, but it's not the norm, so just keep that in mind!
Great start, but I’d recommend this when checking null values: instead of comparing against 0x00, use ``. It’s the convention in C for null characters. Also, reorder your error checks so that you check `cl_array[0]` first. It makes your code cleaner. And regarding the `for` loop for upper/lower case conversion, maybe simplify it with an `if/else` statement instead of using continue; it’ll make it easier to read. Iterating over the entire array isn't necessary after you’ve found the null character, so you can limit the loop to the significant part of your array. Hope this helps!
Thanks for the advice! I’ll definitely apply this when coding next.
I totally agree about the `Yoda conditions` being less clear! It's better to stick with standard comparisons for readability.
Good catch on the scanf issue! I didn’t think about that. I’ll remember to refactor my checks based on your advice.