Предложения по улучшению функции C ReplaceString?

Question

Я только начал изучать C-программирование и был бы признателен за критику в отношении моей функции ReplaceString.Это кажется довольно быстрым (он не выделяет никакой памяти, кроме одного malloc для строки результата), но кажется очень многословным, и я знаю, что это можно сделать лучше.

Пример использования:

printf("New string: %s\n", ReplaceString("great", "ok", "have a g grea great day and have a great day great"));
printf("New string: %s\n", ReplaceString("great", "fantastic", "have a g grea great day and have a great day great"));

Код:

#ifndef uint
    #define uint unsigned int
 #endif

char *ReplaceString(char *needle, char *replace, char *haystack)
{
    char *newString;
    uint lNeedle = strlen(needle);
    uint lReplace = strlen(replace);
    uint lHaystack = strlen(haystack);
    uint i;
    uint j = 0;
    uint k = 0;
    uint lNew;
    char active = 0;
    uint start = 0;
    uint end = 0;

    /* Calculate new string size */    
    lNew = lHaystack;

    for (i = 0; i < lHaystack; i++)
    {

        if ( (!active) && (haystack[i] == needle[0]))
        {
            /* Start of needle found */
            active = 1;
            start = i;
            end = i;
        }
        else if ( (active) && (i-start == lNeedle) )
        {
            /* End of needle */
            active = 0;
            lNew += lReplace - lNeedle;
        }
        else if ( (active) && (i-start < lNeedle) && (haystack[i] == needle[i-start]) )
        {
            /* Next part of needle found */
            end++;
        }
        else if (active)
        {
            /* Didn't match the entire needle... */
            active = 0;
        }

    }
    active= 0;
    end = 0;


    /* Prepare new string */
    newString = malloc(sizeof(char) * lNew + 1);
    newString[sizeof(char) * lNew] = 0;

    /* Build new string */
    for (i = 0; i < lHaystack; i++)
    {

        if ( (!active) && (haystack[i] == needle[0]))
        {
            /* Start of needle found */
            active = 1;
            start = i;
            end = i;
        }
        else if ( (active) && (i-start == lNeedle) )
        {
            /* End of needle - apply replacement */
            active = 0;

            for (k = 0; k < lReplace; k++)
            {
                newString[j] = replace[k];
                j++;
            }
            newString[j] = haystack[i];
            j++;

        }
        else if ( (active) && (i-start < lNeedle) && (haystack[i] == needle[i-start])
                )
        {
            /* Next part of needle found */
            end++;
        }
        else if (active)
        {
            /* Didn't match the entire needle, so apply skipped chars */
            active = 0;

            for (k = start; k < end+2; k++)
            {
                newString[j] = haystack[k];
                j++;
            }

        }
        else if (!active)
        {
            /* No needle matched */
            newString[j] = haystack[i];
            j++;
        }

    }

    /* If still matching a needle... */
    if ( active && (i-start == lNeedle))
    {
        /* If full needle */
        for (k = 0; k < lReplace; k++)
        {
            newString[j] = replace[k];
            j++;
        }
        newString[j] = haystack[i];
        j++;
    }
    else if (active)
    {
        for (k = start; k < end+2; k++)
        {
            newString[j] = haystack[k];
            j++;
        }
    }

    return newString;
}

Есть идеи?Большое спасибо!

Answer 1

Не звоните Стрлену (стогу сена).Вы уже проверяете каждый символ в строке, поэтому вычисление длины строки подразумевается для вашего цикла следующим образом:

for (i = 0; haystack[i] != '\0'; i++)
{
    ...
}
lHaystack = i;

Answer 2

Возможно, вы делаете это по-своему для практики.Если это так, вы получаете много очков за усилия.

Если нет, вы часто можете сэкономить время, используя функции, которые есть в C Runtime Library (CRT), вместо кодирования вашей собственной эквивалентной функции.Например, вы можете использовать strstr , чтобы найти строку, предназначенную для замены.Другие функции работы со строками также могут быть полезны для вас.

Хорошим упражнением было бы завершить этот пример к вашему удовлетворению, а затем перекодировать, используя CRT, чтобы увидеть, насколько быстрее это кодировать и выполнять.

Answer 3

При первом цикле вы должны сохранить индексы того, где должна быть замена, и пропустить те, что указаны в strcopy / replace части функции.Это привело бы к циклу, в котором вы выполняете только strncpy из стога сена или замену новой строки.

Answer 4

Мое одно предложение не имеет ничего общего с улучшением производительности, но с улучшением читабельности.

«Симпатичные» имена параметров гораздо сложнее понять, чем описательные. Какой из следующих параметров вы считаете лучше передать свое назначение?

char *ReplaceString(char *needle, char *replace, char *haystack)
char *ReplaceString(char *oldText, char *newText, char *inString)

С одним вы должны сознательно сопоставить имя с целью. С другой, целью является имя. Жонглирование связями имен в вашей голове при попытке понять фрагмент кода может стать трудным, особенно с увеличением числа переменных.

Может показаться, что это не так важно, когда вы используете код один, но это имеет первостепенное значение, когда ваш код используется или читается кем-то другим. И иногда, «кто-то еще» - это вы сами, год спустя, просматриваете свой собственный код, задаетесь вопросом, почему вы ищете в стогах сена и пытаетесь заменить иглы;)

Answer 5

Задайте параметры const

char *ReplaceString(const char *needle, const char *replace, const char *haystack)

О ... Должна ли функция работать только один раз для слова?

ReplaceString("BAR", "bar", "BARBARA WENT TO THE BAR")