OP's Questions
Is there specifically any way I could improve the argument/input parsing blocks?
See #3 far below
Would it be more consistent to ask the user for minutes if they supply an hours command line argument?
Could go either way on that. #3 far below does that.
Should I offer the variety of options at all, or should I stick to command line arguments only or "dynamic" input only?
Break the needed input into 2 groups: required and optional. An optional parameter may be the format to print the result another way - it is not required to run the core function of code. A required one is the hours. Minutes could go in either group.
If none of the required parameters are supplied, use interactive mode.
If all of the required parameters are supplied, proceed without asking.
If partial required parameters are supplied, error out.
- Are there any meaningful names that could be assigned to the 1, 2, and 3 in the parsing blocks that would make the code more readable?
Not really. Alternatively the argument should be paired with a prefix one. Sample code invocation:
MBM_Minutes -hour 12 -minute 33
same result as
MBM_Minutes -minute 33 -hour 12
Optional arguments could be "-help" "-format AMPM", etc.
Other observations
argv[]
Should code trust that the string will convert to a valid int
number of hours? Look to strtol()
or sscanf()
if in doubt (or experienced).
// hours = atoi(argv[1]);
if (sscanf(argv[1], "%d", &hours) == 1) Success();
else Handle_Failure();
// better strtol() example down below
The last should be a catch-all else
.
// else if(argc != 3) {
else {
printf("Usage: %s (int hours) (int minutes)\n", argv[0]);
From a code layout, often is is best to qualify variables first, then proceed to the normal flow of code. Note: argc == 0
is possible, though rare.
if (argc < 1 || argc > 3) {
printf("Usage: %s (int hours) (int minutes)\n", argv[0]);
// or send to stderr
fprintf(stderrr, "Usage: %s (int hours) (int minutes)\n", argv[0]);
return -1;
}
int hours;
if (argc < 2) {
GetInt("Enter hours:", &hours, 0, 24);
} else {
hours = atoi(argv[1]);
}
int minutes;
if (argc < 3) {
GetInt("Enter minutes:", &hours, 0, 59);
} else {
hours = atoi(argv[1]);
}
User Input
When dealing with user input, avoid being a hacker target. Step 1: check user input function results.
puts("Enter the number of hours:");
if (scanf("%d", &hours) != 1) {
perror("Number not entered\n");
eixt(-1);
}
To be certain the output is truly outputted and not buffered, make certain the output is flushed before attempting input.
puts("Enter the number of hours:");
fflush(stdout);
if (scanf("%d", &hours) != 1) {
Input values should be qualified to the accept range.
if (scanf("%d", &hours) != 1) {
...
}
if (hours < 0 || hours > 24) {
Handle_Range_Error(hours, 0, 24);
...
}
Take user input in a string and then parse it. Easier to cope with bad input. Suggests avoiding scanf()
and use fgets()
.
Common code should exist in a helper function. Putting the above ideas together results in a not-so-simple code and it is certainly overkill for a simple exercise. Yet it is here to give you ideas of what it takes to handle general user input.
// return 0 on success
int GetInt(const char *prompt, int *dest, int min, int max) {
long number;
do {
fputs(prompt);
fflush(stdout);
char buf[100];
if (fgets(buf, sizeof buf, stdin) == NULL) return EOF;
char *endptr;
errno = 0;
long number = strtol(buf, &endptr, 10);
if (endptr == buf) continue; // no conversion
if (errno) continue; //overflow in conversion
if (number < min || number > max) continue; // out of range
while (isspace(*endptr)) endptr++;
} while (*endptr); // extra junk at the end of the string
*dest = (int) number;
return 0;
}
int main(...
if (GetInt("Enter hours:", &hours, 0, 24)) {
Handle_EOF(); // Maybe post an error message and exit
}
Constants
Alternatives to declaring a global object. There are various pros and cons to these. What is best is to code to your group's coding standards. In doubt, go with the last, simplest one.
const unsigned short MinutesPerHour = 60;
// or full width to avoid promote to `int`
const unsigned MinutesPerHour = 60;
// or
#define MinutesPerHour 60
// or insure unsigned-ness
#define MINUTES_PER_HOUR 60u
// or classically
#define MINUTES_PER_HOUR 60
Minor
Spelling: hourss
--> hours