It's a nice piece of work. Short, understandable, to the point.
#include <stdio.h>
#include <stdlib.h>
After the includes, there should be an empty line. It's like having a paragraph in a long prose text, to help the reader.
enum DATE_FORMAT{DASHES,SLASHES};
typedef enum DATE_FORMAT DFORMAT;
Since you don't use the name DATE_FORMAT
, you can also write it in a single line: typedef enum { DASHES, SLASHES } DFORMAT
.
typedef struct {
unsigned char day;
unsigned char month;
unsigned short year;
} DATE;
Technically speaking, you are right. The date components fit in an unsigned char
or an unsigned short
. Nevertheless I would choose unsigned int
for all of the fields, simply because I want to express that the date components are integers. I would only use the smaller types when it comes to hardware-related things.
/* Allocates new DATE with specified values and returns a ptr to it. User must NULL-check */
DATE *create_date(unsigned char month, unsigned char day, unsigned short year)
{
if((month > 12) || (month < 1))
return NULL;
if((day > 31) || (day < 1))
return NULL;
if((year > 9999) || (year < 0))
return NULL;
DATE *the_date = malloc(sizeof(DATE));
if(the_date == NULL)
return NULL;
the_date->day = day;
the_date->month = month;
the_date->year = year;
return the_date;
}
Great code. Well, except for the order of parameters. Almost all other people who designed a similar function used the order year, month, day
. It's simple to remember: from large to small. The American order month, day, year
is confusing to the rest of the world.
void printf_date(DATE *input, DFORMAT format)
{
if(input == NULL)
printf("Invalid DATE.\n");
if(format == DASHES)
printf("%hhu-%hhu-%hu\n",input->month,input->day,input->year);
if(format == SLASHES)
printf("%hhu/%hhu/%hu\n",input->month,input->day,input->year);
return;
}
The return
at the end is redundant. Just remove it.
If you had chosen unsigned int
for the date components, the printf specifiers would be the natural %u
here instead of the distracting %hhu
and %hu
, which is much more common and therefore easier to read.
Since you include the \n
at the end of the output, your function should better be called println_date
. The standard printf
function doesn't append a newline, so neither should your function.
void free_date(DATE *dtf)
{
if(dtf == NULL)
{
printf("Date free failed.\n");
}
else
{
free(dtf);
printf("Date freed successfully\n");
}
return;
}
The return
is redundant.
As soon as your program works, the printf
calls must go away.
You can omit the if
clause, since the free
function can handle a NULL
argument. It does nothing in that case.
The parameter name looks weird. Does it mean date_to_free
? Don't use abbreviations. The name should simply be date
.
So the code should look like:
void free_date(DATE *date) {
free(date);
}
Continuing with your code:
int main()
Just empty parentheses mean unspecified number of arguments for historic reasons. You should write int main(void)
instead.
{
DATE *my_date = create_date(3,17,2017);
if(my_date == NULL)
{
printf("Please provide a valid date next time... exiting.\n");
return(EXIT_FAILURE);
}
Error messages don't belong on stdout
. They should be written to stderr
, like this:
fprintf(stderr, "Please provide ...");
Continuing with your code:
printf_date(my_date, SLASHES);
printf_date(my_date, DASHES);
free_date(my_date);
return(EXIT_SUCCESS);
}
Very nice. But since return
is not a function, it shouldn't look like it. Instead, write return EXIT_FAILURE
without the parentheses.