Replace String Function

Hi everyone. I am new to C, but I recently completed an exercise from a book that finds a string in another string, removes the string, and replaces it with another string. I am happy I finally got the code to work!

I figured I'd post it and see if there are any suggestions/feedback for ways to have done things differently or simplify some of these things?

Code:

---

// Replace String


#include <stdio.h>
#include <stdbool.h>


bool  replaceString (char source[], char s1[], char s2[])
{
   
    int findString (char s1[], char s2[]);
    void    removeString    (char source[], int start, int remove );
    void  insertString (char source[], char insert[], int x);
    int index;
   
    int i;
    bool stillFound;
   
    index = findString(source, s2);
   
    if (index > 0)
    {
        for (i = 0; s1[i] != '\0'; ++i)
        {                                                      //count letters in string to replace
            ;
        }
       
        if (s1[i] == '\0')
        {
            i = 1;
        }
       
        stillFound = true;
       
        removeString(source, index, i);    //if string is found, remove string starting at index location
       
        insertString(source, s1, index);
       
    }
   
    else if (index < 0)
    {
        stillFound = false;
    }
   
    return stillFound;
}


//function to insert string


void insertString (char s1[], char s2[], int x)
{
    int sourceletters;
    int replaceletters;
    int i;
   
    for (sourceletters = 0; s1[sourceletters] != '\0'; ++sourceletters)    //count letters in source, i = letters in source
    {
        ;
    }
   
    for (replaceletters = 0; s2[replaceletters] != '\0'; ++replaceletters)
    {
        ;
    }
   
    for (sourceletters = sourceletters; sourceletters >= x; --sourceletters)
    {
        s1[sourceletters + replaceletters] = s1[sourceletters];
    }
   
    for (i = 0; i < replaceletters; ++i, ++x)
    {
        s1[x] = s2[i];
    }
   
}
//Remove String


void    removeString    (char source[], int start, int remove )
{
    int i;
   
    for (i = start; source[i] != '\0'; ++i)
    {
        source[i] = source [i + remove];
    }
}




//FindString Version 2




int findString (char s1[], char s2[])
{
    int i, j = 0, result = 0;
   
    for (i = 0; s1[i] != '\0'; ++i)
    {
        if (s1[i] != s2[j])
        {
            j = 0;
            result = -1;
        }
       
        if (s1[i] == s2[j])
        {
            ++j;
            if (s2[j] == '\0')
            {
                result = i - (j - 1);
                return result;
            }
        }
    }
   
    return result;
}




int main    (void)
{
    int findString (char s1[], char s2[]);
    void    removeString    (char source[], int start, int remove );
    void  insertString (char source[], char insert[], int x);
    bool  replaceString (char source[], char s2[], char s3[]);
   
    char source[50] = "hello how are you doing today?";
    char s1[] = "";
    char s2[] = " ";
    bool stillFound;
   
    printf("If you replace the character string \"%s\" with the character string \"%s\" in the string \"%s\" the result is ", s2, s1, source);


    do
        stillFound = replaceString(source, s1, s2);
    while ( stillFound );
       
    printf("\"%s\".\n", source);
   
   
    return 0;
   
}

---

The best place to put forward declarations is outside of functions in the global scope. This way you don't need to do forward declarations in every function. In other words, put this:

Code:

---

int findString (char s1[], char s2[]);
void    removeString    (char source[], int start, int remove );
void  insertString (char source[], char insert[], int x);
bool  replaceString (char source[], char s2[], char s3[]);

---

right after your include statements.

Quote:

---

Originally Posted by bithub

The best place to put forward declarations is outside of functions in the global scope. This way you don't need to do forward declarations in every function. In other words, put this:

Code:

---

int findString (char s1[], char s2[]);
void    removeString    (char source[], int start, int remove );
void  insertString (char source[], char insert[], int x);
bool  replaceString (char source[], char s2[], char s3[]);

---

right after your include statements.

---

Hi awesome. Thank you so much. This is exactly what I am referring to, anything I can do to make the code more efficient and more readable.

What you're saying seems like the most logical way to do things, but for some reason the book I am going through has all the declarations in the main function, even if it the main function is the last in the program.

Some of those comments in there were just thrown in there while I was working through the exercises to make sense of what was going on. The comments probably aren't all that helpful.

Quote:

---

Originally Posted by EricRoberts

What? I Didn't mean to quote this. Move along please.

---

Code:

---

#include<stdlib.h>
#include<stdio.h>
#include<sys/stat.h>
int CLenFrom(char *AString,int From);//W
int CPos(char *AString,char *SubString, int Skips);//W
int CLen(char *AString);//W
void CCopy(char *AString,int FromPos_Main,int ToPos,char *ResultBuffer,int FromPos_Result,int IncludeEOL);//W
/*
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Even though a call like CCopy(A,0,0,A,0,1) Seems to work it should not be used.
Saving to the source buffer causes confusion in these functions.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
*/
void CReplaceFrom(char *AString,char *SubString,int FromPos,char *ResultBuffer);//W
void CInsert(char *AString,char *SubString,int AtPos,char *ResultBuffer);//W
void CDelete(char *AString,int FromPos,int ToPos,char *ResultBuffer);//W
int FileExists(char *AString);//W
/*Stat Returns MetaData Of A File In A stat Structure*/


int FileExists(char *AString)
{
return stat(AString,NULL) == 0; //I have no idea if NULL will cause a memory leak.
}


void CDelete(char *AString,int FromPos,int ToPos,char *ResultBuffer)
{
    if(FromPos==0)
        FromPos==1;
    CCopy(AString,0,FromPos-1,ResultBuffer,0,0);
    CCopy(AString,ToPos+1,CLen(AString),ResultBuffer,FromPos,1);
}




void CInsert(char *AString,char *SubString,int AtPos,char *ResultBuffer)
{
    int Length_SubString = CLen(SubString);
    CCopy(AString,0,AtPos,ResultBuffer,0,0);
    CCopy(SubString,0,Length_SubString,ResultBuffer,AtPos,0);
    CCopy(AString,AtPos,CLen(AString),ResultBuffer,AtPos+Length_SubString,1);
}


void CReplaceFrom(char *AString,char *SubString,int FromPos,char *ResultBuffer)
{
    CCopy(AString,0,FromPos,ResultBuffer,0,0); //Stores all chars up and to the replace point in the ResultBuffer
    int i = FromPos;
    int Length_SubString = CLen(SubString);
    int Length_String = CLen(AString);
    int j=0;
    for(i;j!=Length_SubString;i++)
        {
        ResultBuffer[i]=SubString[j++];
        }
    if(AString[i]=='\0')
    {
        ResultBuffer[i]='\0';
        return;
    }


    else
    {
    i--;//A Step Back
    for(i++;i!=Length_String;i++)
    {
    ResultBuffer[i]=AString[i];
    }
    ResultBuffer[i]='\0';
    }






}


void CCopy(char *AString,int FromPos_Main,int ToPos,char *ResultBuffer,int FromPos_Result,int IncludeEOL)
{
//This function does not malloc / realloc / calloc / free;
int i = FromPos_Main;
int j=FromPos_Result;
if (ToPos<FromPos_Main)
    return;
for(i;i!=ToPos+1;i++)
    {
    ResultBuffer[j++]=AString[i];
    }


if (IncludeEOL)
    ResultBuffer[j++]='\0';
return;


}


int CPos(char *AString,char *SubString, int Skips)
{
    int Length_SubString=CLen(SubString);
    int Length_String=CLen(AString);
    int i = 0;
    int Skipped = 0;
    int j;
    int Matches;
    for(i;i!=Length_String;i++) //I will look for the /0 char myself.
    {
    j=0;
    Matches=0;


    if (AString[i]==SubString[j])
    {
        Matches++;
        while((j!=Length_SubString) && (Matches!=Length_SubString))
        {
            j++;
            if(AString[j+i]!=SubString[j])
                {break;}
            else
                {Matches++;}
        }
    if (Matches==Length_SubString)
    {
        Skipped++;
        if (Skipped>Skips)
            return Length_SubString-j+i-1;
    }
        else
        i=i+j-1;
    }
    }
return -1;
}


int CLen(char *AString)
{
    int i = CLenFrom(AString,0);
    return i;
}


int CLenFrom(char *AString,int From)
{
  int i = From;
  for(i;;i++)
  {
    if (AString[i]=='\0')
            {break;}
  }
    return i-From;
}

---

I started programming in delphi which has a copy , insert and delete function for working with strings. They make string handling pretty easy so I decided to rewrite them in C.

The core of all of this is the CCopy function . The params are fairly straight forward but i'll explain anyway.

void CCopy(char *AString,int FromPos_Main,int ToPos,char *ResultBuffer,int FromPos_Result,int IncludeEOL);

AString is simple. FromPos_Main and ToPos are the positions from where to where in AString you wish to copy. ResultBuffer is your resulting string. Please dont do something like CCopy(AString,0,5,AString,0,1) I didn't write the function to work that way nor am I planning on thinking about what'll happen right now. If I remember correctly in some cases it will work fine but in others yeah. Uhm. Just dont .

The FromPos param is the Position in the resulting string where CCopy should start inserting characters . IncludeEOL will simply add a \n if a one is provided - I added a whole lot of additional params to make the insert and delete functions easier.

I bet someone won't agree with something somewhere in my code but I'd be happy to hear from them ^^

Editing for days!

Quote:

---

Originally Posted by RagingGrim

I bet someone won't agree with something somewhere in my code but I'd be happy to hear from them ^^

---

Some things do come to mind:

• You need to improve on your indentation. Some of it is there, but it needs to be consistent, especially when you're nesting if statements within loops, etc.
• Your code is far from const-correct. Take for example CCopy: you obviously are not going to modify what AString points to, so you should have declared it a pointer to const char.
• You should document the interface provided, e.g., in comments before the function declarations/definitions, describe what the function is for and anything special concerning the arguments to be passed to them (such as the pre-conditions).
• FileExists looks like it does not belong. Perhaps it should be moved to a different header/source file, removing the dependency on <sys/stat.h>, which after all is not part of standard C.
• CLen looks redundant: surely strlen will do?
• Have you considered using size_t instead of int for parameters representing an index or a size?

Besides the above, I observed this:

Code:

---

    if(FromPos==0)
        FromPos==1;

---

Clearly, the comparison of FromPos with 1 has no net effect.

CLenFrom also looks pretty convoluted. You could have written:

Code:

---

int CLenFrom(const char *AString, int From)
{
    int i = From;
    while (AString[i] != '\0')
    {
        i++;
    }
    return i - From;
}

---

or even:

Code:

---

int CLenFrom(const char *AString, int From)
{
    return strlen(AString + From);
}

---

The source file isn't really for use I just put functions in there ; Anything that might not be apparent , used often or easily forgotten.

If I remember correctly I used stat because you didn't need permission to read the file to see if it exists something like that. I'll definitely look at my indentation more carefully . I'm not even sure if that was me or codeblocks . Sometimes the IDE does things I don't like - I either leave it that way or edit it out XD Hence all the different indentations.

Thanks for reminding me of size_t . I haven't tested this but couldn't I just say something like CCopy(...,unsigned int,...) . I'll definitely look at the size_t type again because I remember something about it limiting you to signed integers and I'm curious whether it would make a difference or if it's the same.

The frompos == 1 thing was probably me fiddling with the code :3 I rewrote these functions quite a few times :O

Thanks for all the pointers , one or two I'll have to do some googling first though :D

I use an old netbook with a 1024x600 screen, so I have to "fight" with vertical spacing in order to keep as much code as possibile visible. This is why I would have formatted things in a different way. As an example, I would have written the following code of yours...

Code:

---

void insertString (char s1[], char s2[], int x)
{
    int sourceletters;
    int replaceletters;
    int i;
   
    for (sourceletters = 0; s1[sourceletters] != '\0'; ++sourceletters)    //count letters in source, i = letters in source
    {
        ;
    }
   
    for (replaceletters = 0; s2[replaceletters] != '\0'; ++replaceletters)
    {
        ;
    }
   
    for (sourceletters = sourceletters; sourceletters >= x; --sourceletters)
    {
        s1[sourceletters + replaceletters] = s1[sourceletters];
    }
   
    for (i = 0; i < replaceletters; ++i, ++x)
    {
        s1[x] = s2[i];
    }
   
}

---

...this other way:

Code:

---

void insertString (char s1[], char s2[], int x) {
    int i, sourceletters, replaceletters;
   
    for (sourceletters = 0; s1[sourceletters] != '\0'; ++sourceletters);
    for (replaceletters = 0; s2[replaceletters] != '\0'; ++replaceletters);
    for (sourceletters = sourceletters; sourceletters >= x; --sourceletters)
        s1[sourceletters + replaceletters] = s1[sourceletters];
    for (i = 0; i < replaceletters; ++i, ++x) s1[x] = s2[i];
}

---

Not that is any "better", it just tries to compensate the limitations imposed by my hardware.

If you want some tips, use standard libarary functions. Here's how I would've written replace and insert, without optimization:

Code:

---

char * xstrins(char * source, const char * insert, size_t pos)
{
    size_t slen = strlen(source + pos);
    size_t ilen = strlen(insert);

    /* Extend the string the amount required (including \0) */
    memmove(source + pos + ilen, source + pos, slen + 1);
    /* Write the new string in place */
    return memcpy(source + pos, insert, ilen);
}


char * xstrrepl(char * source, const char * target, const char * replace)
{
    char * match = strstr(source, target);

    if (match) {
        size_t tsize = strlen(target);
        /* Delete the match (copy \0 also) */
        memmove(match, match + tsize, strlen(match + tsize) + 1);
        /* Now insert our replacement string */
        xstrins(match, replace, 0);
    } 

    return match;
}

---