Is my C style good? 100 line timer program

This code works exactly as the prompt and the code predict.
Is my style good, my implementations, or what should I change, or what? I'm trying to improve my code, and writing more of it helps... Any and all relevant feedback is good.

```
#include
#include
#include

#define SECONDS_PER_MINUTE 60
#define MINUTES_PER_HOUR 60
#define HOURS_PER_DAY 24
#define PROMPT_STRING "Type any key to show time, q to show time then quit: "

#define TIME_TYPE unsigned long

char getChar() ;
void updateTimes() ;
void initTimes() ;
void printPrompt() ;
void printTime (TIME_TYPE timeInit) ;

TIME_TYPE start, end;

int main (const char argc, char* argv[]) {
char command = 0;
initTimes();
while (command != 'q') {
printPrompt();
command = getChar();
updateTimes();
printTime(end-start);
}
return 0;
}

char getChar() {
char retVal = -1;
char trashVal = -1;
retVal = getc(stdin);
while ( trashVal != '\n' && retVal != '\n') {
trashVal = getc(stdin);
}
return retVal;
}

void updateTimes() {
end = time(0);
}

void initTimes() {
start = time(0);
end = start;
}

void printPrompt() {
printf(PROMPT_STRING);
}

void printTime (TIME_TYPE timeInit) {
/ Basic declarations /
TIME_TYPE time = timeInit;
int days, hours, minutes, seconds;
int timeDifferential = 0;
/ Assigning crap, reassigning remaining seconds to 'time' /
/ Days /
timeDifferential = (SECONDS_PER_MINUTE *
MINUTES_PER_HOUR *
HOURS_PER_DAY);
days = (time/timeDifferential);
time -= timeDifferential*days;
/ Hours /
timeDifferential = (SECONDS_PER_MINUTE *
MINUTES_PER_HOUR);
hours = (time/timeDifferential);
time -= timeDifferential*hours;

Compiler Warnings

Firstly, always enable maximum warning settings for your compiler, it will catch things that you miss. For example, with gcc -Wall -Wextra, the following is produced:


timer.c:20:5: warning: first argument of 'main' should be 'int' [-Wmain]


timer.c: In function 'main':
timer.c:20:22: warning: unused parameter 'argc' [-Wunused-parameter]
timer.c:20:34: warning: unused parameter 'argv' [-Wunused-parameter]


timer.c: In function 'printTime':
timer.c:92:21: warning: format '%u' expects argument of type 'unsigned int', but argument 2 has type 'long unsigned int' [-Wformat]

Nothing major, which is good, but we can fix some of these up. Firstly, as it says, main should have signature:

int main (int argc, char* argv[])

not the current signature of

int main(const char argc, char* argv[])

The next warnings complain about unused parameters of argc and argv. If you aren't going to utilize command line arguments in your program, you can simply leave them out:

int main()

Fixing the printf warning is a simple matter of replacing the %u\n with %lu\n.

Code Style

Generally, there is too much whitespace. Very few programmers use 8 space indentation - I'd recommend sticking to 4. Also, leaving a space between method declarations and the trailing semicolon is slightly odd - char getChar() ;. None of this is wrong, it's just unorthodox, and will probably be a bit jarring to most C programmers (and programmers are a picky bunch). Pretty much the first thing I did with your code is modify the indentation and remove any whitespace for trailing semicolons.

Never, ever #define a type. Utilize typedef instead. This also means types should not be ALL_CAPS.

typedef unsigned long time_type;

There are very good reasons for this. For example, if we define a pointer type:

#define ptr_int int *

Now, what happens if we do the following:

ptr_int x, y;

The preprocessor will expand this to:

int *x, y; //Uh-oh!

This is declaring a pointer to integer (x) and a normal integer (y). This is sure to introduce annoying and hard to track down bugs. If instead we used a typedef, this will fix the above problem (although it shows you one must be careful when having more than declaration per line).

Comments are sparse (then again, this program probably doesn't really need them). However, you've fallen into the "obvious comment" trap:

/* Basic declarations */
/* Assigning crap, reassigning remaining seconds to 'time' */
/* Days */

and so on. None of these is really helpful. We can see that up the top the basic declarations happen, and that then some assignment takes place. The variable names of days, minutes and seconds are descriptive enough that they make the comments superfluous.

Other Quibbles

getChar actually returns an int. Pretty much anywhere you have a char, you should replace it with an int:

int getChar()
{
    int retVal = -1;
    int trashVal = -1;
    ...
}

int main() 
{
    int command = 0;
    ....
}

Also, why getc(stdin) over simply getchar()? Either is fine really, but getchar is slightly more idiomatic when reading from stdin.

With any slightly more modern version of C, prefer const declarations to using #define. It's good that you didn't simply write magic numbers all over the place, but prefer things like:

const int seconds_per_minute = 60;

to their #defined counterparts. (They should probably also be static, but don't get too hung up on this while learning).

There are a number of reasons for this. The biggest reason is the fact that #define is just a dumb text replacement mechanism. Say you have a larger program and you are trying to debug it - since #define simply replaces a textual pattern with a given value, all symbols are lost. It's great in the source code to not have any magic numbers, but in a debugger, you won't have that luxury with #define - it'll be back to magic numbers all over again.

The other more minor reasons are that const guarantees it won't change, and if you need to take the address of any of these variables for any reason, well, you're totally out of luck with #define.

That being said, you cannot always get away with using const instead of #define. There's a big post about this on StackOverflow that is worth reading.

Finally, try to avoid using global variables (variables outside the scope of any function, so TIME_TYPE start, end;). In a small program like this it doesn't matter too much, but it's a good habit to get into.

I've typed a bit of a wall of text, but most of this stuff is pretty minor. Summing up:

• Always compile with all warnings enabled.

• Prefer 4 space to 8 space indentation.

• Don't use #define to introduce a new type or a type alias; use typedef.

• Prefer to use const variables to #define'd variables.

• Try to avoid global variables if possible.

StackExchange Code Review Q#25921, answer score: 11