0

I'm working on a function that accepts 3 C-strings and my objective is to replace the word that the user finds anywhere on the sentence with the one he/she inputs.

Below is the code of my function: 

void replaceSubstring(char str1[], char str2[], char str3[])
{
    char *a= new char(sizeof(str1));
    char *b= new char(sizeof(str1));

    int len=0;
    for(unsigned int ind= 0; ind< sizeof(str1); ind++)
    {
        for(unsigned ind1= 0; ind1< sizeof(str2); ind1++)
            a[ind]= str1[ind+ ind1];
            a[ind]= '\0';

            if(strcmp(str1, str2))
            {
                for(unsigned int y= 0; y< sizeof(str3); y++)
                {
                    b[len]= str3[y];
                    len++;
                }
                    ind+= (sizeof(str2)- 1);
            }
            else
            {
                cout<< "Error! No match found!"<< endl;
            }
    }
    cout<< b<< endl;
}

Let us show an example of my output:

  1. Enter a string: I love mangoes

  2. Enter the word you want to look for: mangoes

  3. Enter the new word to replace "mangoes" with: cheese

  4. Output? chee─

Can anyone explain what can I do to improve this and why this is causing a glitch? Any info is truly appreciated.

P.S: I've tried working with strstr, but strstr returns the pointer and leaves the sentence cut off and then I get worried figuring out a way to skip characters from the end of the sentence to where the word is and figuring out what to do afterwards. How could I do this using strstr, if possible, without cutting the sentence?

Thank you very much for your help.

Robin Painter
  • 3
  • 1
  • http://stackoverflow.com/questions/1328223/sizeof-array-passed-as-parameter Also use `std::string` – Baum mit Augen Jul 23 '15 at 12:59
  • if you tag it c++ you should use c++. If you want to use low level primitives like char array read your textbook about pointers first. – Slava Jul 23 '15 at 13:04
  • you're leaking memory. The memory pointed to by `a` and `b` is never freed. – Captain Obvlious Jul 23 '15 at 13:13
  • `a` and `b` point to one `char` each. The `sizeof` a pointer will be 4 or 8 - all your `sizeof`s are wrong. Read about strings, arrays, and pointers in your fine book. – molbdnilo Jul 23 '15 at 13:30

2 Answers2

0

You compare str1 and str2 with stcmp instead of a and str2 . Strcmp returns 0 (= false) when the strings are equal. b is only completed with chars from str3, and it does not recopy anything from str1.

Tuirenen
  • 61
  • 1
  • 9
0

You are using low level primitives from C, where array in form you used it is a synonym to a pointer. So your function signature is equivalent to:

void replaceSubstring(char *str1, char *str2, char *str3);

According to this all usages of sizeof() in your code is incorrect, instead of length of the string as you assumed it gives you size of a pointer on your platform. You also have a memory leak as you allocate memory for 2 pointers and never release it:

char *a= new char(sizeof(str1));
char *b= new char(sizeof(str1));

You should use std::string which has proper memory management and string manipulation methods.

Slava
  • 43,454
  • 1
  • 47
  • 90
  • Thank you @Slava. You're right, using strings is more efficient, but I'm trying to teach myself on how to work at a low level. Now I think that since Tuirenen says that "b is only completed with chars from str3, and it does not recopy anything from str1" , do you think I should append b to str1, and concatenate, or would it be best that str3 = str2 and then append it? – Robin Painter Jul 23 '15 at 13:43
  • @RobinPainter it is not quite clear what you are trying to do, why you allocate `a` and `b` etc. First of all it is not simple question how long result string would be, it depends if `str2` is shorter than `str3` or longer, and how many you need to replace. And you seem not understand how low level strings work in principle, read textbook first. – Slava Jul 23 '15 at 13:51