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
argv. If you aren't going to utilize command line arguments in your program, you can simply leave them out:
printf warning is a simple matter of replacing the
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.
#define a type. Utilize
typedef instead. This also means types should not be
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
seconds are descriptive enough that they make the comments superfluous.
getChar actually returns an
int. Pretty much anywhere you have a
char, you should replace it with an
int retVal = -1;
int trashVal = -1;
int command = 0;
getc(stdin) over simply
getchar()? Either is fine really, but
getchar is slightly more idiomatic when reading from
With any slightly more modern version of
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;
#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
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
- Prefer to use
const variables to
- Try to avoid global variables if possible.