Extracting and reformatting dates from text

Question

The task is find by pattern d--m--y---- all occurrences in text, reformat it to yyyy:mm:dd and print it separately from initial text. Actually I done all except formatting. And even in my written code I have some doubts because I just begun to learn this language. Maybe someone would help me and say what's wrong with my code and how to finish that task on a better way. Any comments are appreciated.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define MAXLINE 1000
#define CONT 1000000

int getlin(char *, int);
int pickDates(char *, char *);
int isDate(char *, int);

int main() 
{
    int len;
    char line[MAXLINE];
    char accum[CONT];
    char *dates = malloc(sizeof(char *) * CONT);

    while ((len = getlin(line, MAXLINE)) > 0)
        strcat(accum, line);

    pickDates(dates, accum);
    printf("%s\n", dates);

    free(dates);

    return 0;
}

int getlin(char *s, int lim) {
    int i, c, j = 0;

    for (i = 0; (c = getchar()) != EOF && c != '\n'; ++i)
    if (i < lim - 2) 
    {
        s[j] = c;
        ++j;
    }
    if (c == '\n')
    {
        s[j] = c;
        ++i;
        ++j;
    }
    s[j] = '\0';

    return i;
}

int pickDates(char *dates, char *cont)
{
    int j = 0;
    const char *template = "d--m--y----";
    char *date;
    int temp_len = strlen(template);

    for (int i = 0; i < strlen(cont); i++)
        if (cont[i] == template[0] && cont[i+3] == template[3] && cont[i+6] == template[6])
        {
            date = malloc(sizeof(char) * temp_len);
            memcpy(date, &cont[i], temp_len);
            if (isDate(date, temp_len))
            {
                j += 8;
                strcat(dates, date);
            }
            free(date);
        }

    dates = realloc(dates, sizeof(char) * j);

    return 0;
}

int isDate(char *date_str, int len)
{
    int i = 0;
    int dd = atoi(&date_str[i+1]);
    int mm = atoi(&date_str[i+4]);
    int yy = atoi(&date_str[i+7]);
    char tmp[5] = {'0'};

    memset(date_str, 0, len - 3);
    date_str = realloc(date_str, sizeof(char) * (len - 3));

    if (dd < 32 && dd > 0 && mm < 13 && mm > 0 && yy > 0)
    {
        sprintf(tmp, "%04d", yy);
        strcat(date_str, tmp);
        sprintf(tmp, "%02d", mm);
        strcat(date_str, tmp);
        sprintf(tmp, "%02d", dd);
        strcat(date_str, tmp);

        return 1;
    }
    else return -1;
}

the posted code produces MANY warnings when run through the compiler. When compiling, always enable the warnings, then fix those warnings. (for gcc, at a minimum use: -Wall -Wextra -Wconversion -pedantic -std=gnu11 ) Note other compilers use different options to produce the same results — user3629249
– user3629249, Commented Dec 8, 2018 at 0:00
regarding: char accum[CONT]; this is a mighty large array to be placing on the stack. Suggest moving to 'file' scope (I.E. outside of any function — user3629249
– user3629249, Commented Dec 8, 2018 at 0:02
OT: regarding: char *dates = malloc(sizeof(char *) * CONT); when calling any of the heap allocation functions: malloc calloc realloc, always check (!=NULL) the returned value to assure the operation was successful — user3629249
– user3629249, Commented Dec 8, 2018 at 0:04
in function: getlin() strongly suggest including braces '{' and '}' around the body of the for() statement — user3629249
– user3629249, Commented Dec 8, 2018 at 0:06
regarding: strcat(accum, line); the function strcat() searches for a NUL byte, then appends the line beginning at the NUL byte. However, the posted code does not set the first byte in accum[] to '\0', so it is unknown where the line will actually be appended. This is undefined behavior and must be corrected — user3629249
– user3629249, Commented Dec 8, 2018 at 0:12

Reinderien · Accepted Answer · 2018-12-08 01:07:47Z

Whenever declaring a function that you only expect to use in the current file, mark it static.

The (anonymous) user in the comments above is correct to indicate that accum is quite large; however, rather than allocating it statically as a global I'd suggest that it be allocated from the heap (malloc).

printf("%s\n", dates) is equivalent to puts(dates), but the latter is more efficient.

Having to predeclare variables in C hasn't been needed since the 90s. Do not predeclare variables. Declare them where they're used; i.e.

for (int i = 0;

That loop doesn't do what you think it does. You're missing braces. The loop will only apply to the first if.

j += 8

Where does 8 come from? Declare this as a constant. Magic numbers are bad.

It's pointless to have a return value for pickDates, so make it void.

date_str in isDate should be a const char * because you shouldn't be modifying it. The same applies to dates in pickDates.

Later, where you have a series of sprintf / strcat, do not use strcat, nor a tmp array. You can write to date_str via a temporary pointer that you increment based on the return value of sprintf.

I suggest inverting this logic:

if (dd < 32 && dd > 0 && mm < 13 && mm > 0 && yy > 0)
{
    // ...
    return 1;
}
else return -1;

to

if (dd < 0 || dd > 31 || mm < 0 || mm > 12 || yy < 0)
    return -1;
// ...
return 1;

Also, if that return value indicates success or failure, you should use a boolean from stdbool.h and use true/false instead of an integer.

This:

int i = 0;
int dd = atoi(&date_str[i+1]);
int mm = atoi(&date_str[i+4]);
int yy = atoi(&date_str[i+7]);

doesn't make a whole lot of sense; i might as well not exist and you might as well do

int dd = atoi(date_str + 1),
    mm = atoi(date_str + 4),
    yy = atoi(date_str + 7);

Stack Exchange Network

Extracting and reformatting dates from text

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Extracting and reformatting dates from text

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions