Longest Common Suffix Is Producing Garbage Values-CodePudding

I'm having trouble writing a program that gets two words and finds the longest common suffix. Whenever I try to copy the contents from the words into my new string "suffix" I keep getting garbage values and I am struggling to find out where I went wrong. Any and all feedback is appreciated!

void longest_suffix(char word1[], char word2[]){
    char suffix[20];
    int count = 0;

    int end = strlen(word2) - 1;

    int j = 0;
    for(int i = strlen(word1)- 1; i >= 0; i--){
        if(word1[i] == word2[end - j]){
            count  ;
        }
        j  ;
    }


    int z = 0, k = 0;
    for(k = 0; k < count; k  ){
        suffix[k] == word2[end - count   z];
        z  ;
    }
    suffix[k] = '\0';
    printf("%s", suffix);
}

CodePudding user response：

I would approach this slightly differently. There is no need to copy the string. If there's a common suffix it's already present in word1 and word2 and it's already null-terminated.

void longest_suffix(char word1[], char word2[]){
    char *p1 = word1   strlen(word1) - 1;
    char *p2 = word2   strlen(word2) - 1;
    char *suffix = NULL;

    while (p1 >= word1 && p2 >= word2) {
        if (*p1 == *p2) {
            suffix = p1;
        }
        p1--;
        p2--;
    }

    printf("suffix: %s\n", suffix);
}

But also: this function signature is not great. And you're printing from a function that is not obviously going to print the output rather than return the results. Consider:

const char *longest_suffix(const char *word1, const char *word2) {
    const char *p1 = word1   strlen(word1) - 1;
    const char *p2 = word2   strlen(word2) - 1;
    const char *suffix = NULL;

    while (p1 >= word1 && p2 >= word2) {
        if (*p1 == *p2) {
            suffix = p1;
        }
        p1--;
        p2--;
    }

    return suffix;
}

I would not print or allocate from such a function. If the caller wants a copy, they can call strdup on the result (if it's non-NULL).

CodePudding user response：

Here's a solution:

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

#define MIN(x, y) (((x) < (y)) ? (x) : (y))

static void longest_suffix(const char word1[], const char word2[])
{
    int len1 = strlen(word1);
    int len2 = strlen(word2);
    int minlen = MIN(len1, len2);
    int i;
    assert(word1[len1] == word2[len2] && word1[len1] == '\0');
    for (i = 1; i <= minlen; i  )
    {
        if (word1[len1 - i] != word2[len2 - i])
            break;
    }
    printf("[%s] = ", &word1[len1 - i   1]);
    printf("[%s]: ", &word2[len2 - i   1]);
    printf("[%s] vs [%s]\n", word1, word2);
    assert(strcmp(&word1[len1 - i   1], &word2[len2 - i   1]) == 0);
}

int main(void)
{
    longest_suffix("abc.c", "def.c");
    longest_suffix("basename.c", "dirname.c");
    longest_suffix("spooky spells - abracadabra", "abracadabra");
    longest_suffix("abracadabra", "spooky spells - abracadabra");
    longest_suffix("xyz", "xyz");
    longest_suffix("c", "c");
    return 0;
}

On the whole, it would be better to separate determining the longest suffix from printing it. Indeed, you should very often separate I/O from the calculations. It makes it easier to reuse the calculation when the I/O requirements change.

Output:

[.c] = [.c]: [abc.c] vs [def.c]
[name.c] = [name.c]: [basename.c] vs [dirname.c]
[abracadabra] = [abracadabra]: [spooky spells - abracadabra] vs [abracadabra]
[abracadabra] = [abracadabra]: [abracadabra] vs [spooky spells - abracadabra]
[xyz] = [xyz]: [xyz] vs [xyz]
[c] = [c]: [c] vs [c]