Skip to main content
Add "more KISS'ing" section about argc & argv
Source Link
Fe2O3
  • 2.3k
  • 1
  • 2
  • 14

More KISS'ing

int main(int argc, char **argv) { // #1
                                  // or
int main( void ) {                // #2

Which of the above is simpler for the reader to scan?

The compiler should have (again) warned you that two parameter variables are unused. The compiler is your friend and really wants to help you write the best code you can. Turn up its warning level and heed its critiques.

The compiler can really only validate the syntax of code, but, in this case, the warning strays into semantics. The presence of those two parameters implies they will have meaning to the function, but they don't! Is something not right in this function?? Consider this warning a bonus from the compiler!


Consistency

        exit (EXIT_FAILURE);
        ....
        exit (0);  // EXIT_SUCCESS???

Consistency

        exit (EXIT_FAILURE);
        ....
        exit (0);

More KISS'ing

int main(int argc, char **argv) { // #1
                                  // or
int main( void ) {                // #2

Which of the above is simpler for the reader to scan?

The compiler should have (again) warned you that two parameter variables are unused. The compiler is your friend and really wants to help you write the best code you can. Turn up its warning level and heed its critiques.

The compiler can really only validate the syntax of code, but, in this case, the warning strays into semantics. The presence of those two parameters implies they will have meaning to the function, but they don't! Is something not right in this function?? Consider this warning a bonus from the compiler!


Consistency

        exit (EXIT_FAILURE);
        ....
        exit (0);  // EXIT_SUCCESS???
Add splats admonishment
Source Link
Fe2O3
  • 2.3k
  • 1
  • 2
  • 14

And, (I'm not going to test this) playing a second game that is offered would replace the original (cooked mode) terminal configuration, that has been saved, with the raw mode configuration that is still in effect from the first game played.. atexit() is nice, but it's NOT going to achieve what the coder thinks it does. Oops!

As written, main() is too trivial, and evaluate() is where all the setup/teardown action happens. Suggest moving the guts of the latter into the former to have fewer functions to deal with. menu() simply interacts with the user, and returns a valid value (or, perhaps -1 to signify termination desired).

    char *name; // style 1
    ...
    char* found; // style 2

The placements of splats, in this code, isn't important... until it becomes important (i.e.: a bug arises.)
Recommend ALL splats be immediately adjacent to the variable name (style 1) and NOT joined to the datatype (style 2).

And, (I'm not going to test this) playing a second game that is offered would replace the original (cooked mode) terminal configuration, that has been saved, with the raw mode configuration that is still in effect from the first game played... Oops!

As written, main() is too trivial, and evaluate() is where all the setup/teardown action happens. Suggest moving the guts of the latter into the former to have fewer functions to deal with. menu() simply interacts with the user, and returns a valid value (or, perhaps -1 to signify termination desired).

And, (I'm not going to test this) playing a second game that is offered would replace the original (cooked mode) terminal configuration, that has been saved, with the raw mode configuration that is still in effect from the first game played. atexit() is nice, but it's NOT going to achieve what the coder thinks it does. Oops!

As written, main() is too trivial, and evaluate() is where all the setup/teardown action happens. Suggest moving the guts of the latter into the former to have fewer functions to deal with. menu() simply interacts with the user, and returns a valid value (or, perhaps -1 to signify termination desired).

    char *name; // style 1
    ...
    char* found; // style 2

The placements of splats, in this code, isn't important... until it becomes important (i.e.: a bug arises.)
Recommend ALL splats be immediately adjacent to the variable name (style 1) and NOT joined to the datatype (style 2).

added 253 characters in body
Source Link
Fe2O3
  • 2.3k
  • 1
  • 2
  • 14

The code is clean and quite readable. This makes it highly suspicious and worthy of very close scrutiny...


Bug report:

void evaluate(char* set, ...
        ...
        memcpy(found, set, sizeof(set));

A common mistake is taking the sizeof a pointer thinking it is the count of elements in an array.
The "slush" of heap allocation of small quantities has prevented a SIGSEGV crash.
The remedy is super-simple, and left as an exercise.


Unnecessary text

The function prototypes at the top are probably unnecessary and should be removed.
Their presence, when main() is at the bottom, suggests there may be a mutual function call operation that is not present in this program.
KISS!


Should have had warning

void set_raw_mode(void) {
    struct termios tattr;
    char *name;

What is the unused name doing there? Exceedingly minor glitch (typo) of no consequence, but...

More of a concern is that the terminal enters raw mode and stays there even though i/o goes back to using both getchar() and scanf() for subsequent games. This may create some surprises for the user (eg: backspace doesn't work anymore??).

And, (I'm not going to test this) playing a second game that is offered would replace the original (cooked mode) terminal configuration, that has been saved, with the raw mode configuration that is still in effect from the first game played... Oops!


Consistency

        exit (EXIT_FAILURE);
        ....
        exit (0);

As written, main() is too trivial, and evaluate() is where all the setup/teardown action happens. Suggest moving the guts of the latter into the former to have fewer functions to deal with. menu() simply interacts with the user, and returns a valid value (or, perhaps -1 to signify termination desired).


Don't do things just because you can

There's a ceiling of 10 "pegs" that can be played.
KISS, again!
Use all-or-part-of simple 10 byte arrays on the stack instead of complicating things with heap allocation/free invitations for bugs to arise.


Learning...

Here is a skeleton version of the same game.
Much can be gained from reading and understanding alternative approaches to solving the same problem.
With this challenge fresh in your mind, and having climbed-the-hill in your own way (good!), your brain is now fertile ground to learn and assimilate alternative techniques and practices.
Always try to enjoy what you do!

The code is clean and quite readable. This makes it highly suspicious and worthy of very close scrutiny...


Bug report:

void evaluate(char* set, ...
        ...
        memcpy(found, set, sizeof(set));

A common mistake is taking the sizeof a pointer thinking it is the count of elements in an array.
The "slush" of heap allocation of small quantities has prevented a SIGSEGV crash.
The remedy is super-simple, and left as an exercise.


Unnecessary text

The function prototypes at the top are probably unnecessary and should be removed.
Their presence, when main() is at the bottom, suggests there may be a mutual function call operation that is not present in this program.
KISS!


Should have had warning

void set_raw_mode(void) {
    struct termios tattr;
    char *name;

What is the unused name doing there? Exceedingly minor glitch (typo) of no consequence, but...

More of a concern is that the terminal enters raw mode and stays there even though i/o goes back to using both getchar() and scanf() for subsequent games. This may create some surprises for the user (eg: backspace doesn't work anymore??).


Consistency

        exit (EXIT_FAILURE);
        ....
        exit (0);

As written, main() is too trivial, and evaluate() is where all the setup/teardown action happens. Suggest moving the guts of the latter into the former to have fewer functions to deal with. menu() simply interacts with the user, and returns a valid value (or, perhaps -1 to signify termination desired).


Don't do things just because you can

There's a ceiling of 10 "pegs" that can be played.
KISS, again!
Use all-or-part-of simple 10 byte arrays on the stack instead of complicating things with heap allocation/free invitations for bugs to arise.


Learning...

Here is a skeleton version of the same game.
Much can be gained from reading and understanding alternative approaches to solving the same problem.
With this challenge fresh in your mind, and having climbed-the-hill in your own way (good!), your brain is now fertile ground to learn and assimilate alternative techniques and practices.
Always try to enjoy what you do!

The code is clean and quite readable. This makes it highly suspicious and worthy of very close scrutiny...


Bug report:

void evaluate(char* set, ...
        ...
        memcpy(found, set, sizeof(set));

A common mistake is taking the sizeof a pointer thinking it is the count of elements in an array.
The "slush" of heap allocation of small quantities has prevented a SIGSEGV crash.
The remedy is super-simple, and left as an exercise.


Unnecessary text

The function prototypes at the top are probably unnecessary and should be removed.
Their presence, when main() is at the bottom, suggests there may be a mutual function call operation that is not present in this program.
KISS!


Should have had warning

void set_raw_mode(void) {
    struct termios tattr;
    char *name;

What is the unused name doing there? Exceedingly minor glitch (typo) of no consequence, but...

More of a concern is that the terminal enters raw mode and stays there even though i/o goes back to using both getchar() and scanf() for subsequent games. This may create some surprises for the user (eg: backspace doesn't work anymore??).

And, (I'm not going to test this) playing a second game that is offered would replace the original (cooked mode) terminal configuration, that has been saved, with the raw mode configuration that is still in effect from the first game played... Oops!


Consistency

        exit (EXIT_FAILURE);
        ....
        exit (0);

As written, main() is too trivial, and evaluate() is where all the setup/teardown action happens. Suggest moving the guts of the latter into the former to have fewer functions to deal with. menu() simply interacts with the user, and returns a valid value (or, perhaps -1 to signify termination desired).


Don't do things just because you can

There's a ceiling of 10 "pegs" that can be played.
KISS, again!
Use all-or-part-of simple 10 byte arrays on the stack instead of complicating things with heap allocation/free invitations for bugs to arise.


Learning...

Here is a skeleton version of the same game.
Much can be gained from reading and understanding alternative approaches to solving the same problem.
With this challenge fresh in your mind, and having climbed-the-hill in your own way (good!), your brain is now fertile ground to learn and assimilate alternative techniques and practices.
Always try to enjoy what you do!

Source Link
Fe2O3
  • 2.3k
  • 1
  • 2
  • 14
Loading