Home made concatenate function

Question

I am trying to create a concatenate function in language C and I am beginner ...

But I have an issue, my function concatenate only the first character of the second string but doesn't work for the all string.

Example :

String1 = "ABCD" and String2 = "EFGH" The result will be : String1 = "ABCDE"

Here my code :

int main(int argc, char *argv[]) 
{   
    char *myChaine1 = (char*)malloc(100);
    char *myChaine2 = (char*)malloc(100);

    myStrCpy("ABCD", myChaine1);
    myStrCpy("EFGH", myChaine2); 

    myStrCat(myChaine1, myChaine2);

    printf("%s", myChaine1);

    free(myChaine1);
    free(myChaine2);
    return 0;
}

// Home made string length function
int myStrLen(const char* word)
{
    int length = 0;
    while (word[length] != '\0')
    {
        length++;
    }
    return length;
}

// Home made Copy string function    
char* myStrCpy(const char* word, char* copy)
{
    int i = 0;
    while (i < myStrLen(word))
    {
        copy[i] = word[i];
        i++;
    }   
}

// Home made concatenate function
char* myStrCat(char* chaine1, const char* chaine2)
{
    int i = 0;
    while (i < myStrLen(chaine2))
    {   
        chaine1[i + myStrLen(chaine1)] = chaine2[i];
        i++;
    }
}

Someone understand where is the issue?

`myStrCpy` and `myStrCat` both fail to copy (or write) the string terminator. They should return a value, but don't. They are also both inefficient: `myStrLen` needs to be called only one, not in every loop iteration. — Weather Vane, Jun 17 '18 at 13:46
In addition to adding the null terminator to the end of the destination strings, your code is also calling myStrLen numerous times, which is iterating over the string again for each time through your loops. This is very inefficient. You should figure out how to get rid of these extra calls. — bruceg, Jun 17 '18 at 13:52
As @WeatherVane said, I modify my concatenate function to not calling myStrLen in the loop but before — Malus_khanot, Jun 17 '18 at 13:56
[don't cast the result of malloc in C](https://stackoverflow.com/q/605845/995714) — phuclv, Jun 17 '18 at 14:02
Not the issue you are after, but still: Your string functions' parameter lists are inconsistently designed: One function takes the destination 1st, the other 2nd. This is confusing and therefore error-prone. — alk, Jun 17 '18 at 16:48
Also for indexing arrays typically, the type `size_t` f used. — alk, Jun 17 '18 at 16:51
Last not least, in Production code (at least) you *always* want to test the outcome of relevant function calls, `malloc()` in your case. It might very well fail and return a dump `NULL`. — alk, Jun 17 '18 at 16:53

score 1 · Answer 1 · answered Jun 17 '18 at 13:56

1

Not sure it’s the answer but neither of your copy or concat routines seem to write ‘\0’ at the end of the output string. You could correct this by using <= on your length check, or you could loop until you get the 0 char and then add it after the loop.

answered Jun 17 '18 at 13:56

user2771365

132
1
7

I agree about the style of caluculating indexed values only once as it makes the code clear. However it’s not catestophic because modern c compilers will optimise your sloppy coding (unless you switch optimisation off, of course). BTW the modified code I see still doesn’t seem to copy the terminating 0 to the output string. – user2771365 Jun 17 '18 at 14:14
Thank you for your messages, with the modification I did it's seems to be better ? I added the '\0' at the end of the string ... – Malus_khanot Jun 17 '18 at 14:24

Malus_khanot · Answer 2 · 2018-06-17T14:23:43.363

1

As @WeatherVane suggested, I changed my function as following :

char* myStrCat(char* chaine1, const char* chaine2)
{
    int i = 0;
    int lastValue = myStrLen(chaine1);
    while (i < myStrLen(chaine2))
    {   
        chaine1[i + lastValue] = chaine2[i];
        i++;
    }
    chaine1[myStrLen(chaine1)] = '\0';
}

The issue was in the iteration of myStrLen in the loop.

edited Jun 17 '18 at 14:23

answered Jun 17 '18 at 14:02

Malus_khanot

45
8

Home made concatenate function

2 Answers2