Nsryjdtyk

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("%sn", 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] = '';



    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;

}

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("%sn", 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);