7
\$\begingroup\$

Are there any other more exotic ways ... or is this one just good?

void strdev (char *str, long from, long to)
{
    char *part1 = malloc(strlen(str) - strlen(&str[from]));
    long i;

    part1[from] = '\0';

    //for(i=0; i<from; i++) part1[i] = str[i]; // Old copying
    memmove(part1, str, from); // New copying
    sprintf(str, "%s%s", part1, &str[to+1]);
    free(part1);
}
\$\endgroup\$
  • 4
    \$\begingroup\$ Define "exotic". Code-obfusticated, some weird library, wings of a butterfly? \$\endgroup\$ – Yann Oct 17 '14 at 15:33
  • \$\begingroup\$ @Yann4 No, It's not a library or wings of a butterfly. It is code. Exotic code. I think it is pretty self-explaining. \$\endgroup\$ – Edenia Oct 17 '14 at 15:37
  • 1
    \$\begingroup\$ I meant, when you ask for a more "exotic" method, what is it in particular that you're looking for? Are you just wanting a general suggestion based upon performance and style? I'm asking for clarity because "exotic" seemed like an odd word to choose. \$\endgroup\$ – Yann Oct 17 '14 at 15:42
  • \$\begingroup\$ I think using memmove instead of manually copying would be easier, then still add the '\0' at the end. \$\endgroup\$ – ferada Oct 17 '14 at 15:43
  • \$\begingroup\$ @Yann4 I know what did you meant. Check definitions of "exotic" in any dictionary. (e.g thefreedictionary.com/exotic) The only definition that fits perfectly for the sense of the question (and what i meant) is 2. \$\endgroup\$ – Edenia Oct 17 '14 at 15:47
10
\$\begingroup\$

Interface

To design functions that other programmers will find useful, it helps to draw inspiration from conventions established by existing string libraries.

In C, as in other languages with array indices numbered from 0, it is customary to use inclusive-exclusive ranges. from would be the index of the first byte to delete; to would be the index of the first byte to keep. That has the nice property that to - from indicates the number of characters to be deleted. That convention would echo the String.substring(beginIndex, endIndex) function that Java offers, for example.

Another common convention in C would be to accept parameters (str, from, len) — the last parameter specifying the number of bytes to remove.

Finally, you should study Perl's splice function and JavaScript's array.splice(), which are generalized versions of this function.

Implementation

Starting from the top…

void strdev (char *str, long from, long to)

The name strdev doesn't mean anything to me. I suggest splice_away.

    char *part1 = malloc(strlen(str) - strlen(&str[from]));

You call strlen() twice, when you don't need to call it at all. malloc(from) will do. Since strlen() works by visiting every byte in the string, you should avoid calling it carelessly.

    long i;

i is no longer used. Prune the dead code.

    memmove(part1, str, from); // New copying

part1 is newly allocated memory, and is guaranteed not to overlap with the source string. Therefore, memcpy() will do.

    sprintf(str, "%s%s", part1, &str[to+1]);

Whoa! Why are you overwriting the initial part of the string with the same characters? You didn't need to do any of that previous work.

The whole function can be condensed down to

char *splice_away(char *str, int from, int to)
{
    memmove(str + from, str + to, 1 + strlen(str + to));
    return str;
}

I've used the inclusive-exclusive range convention mentioned above, and added return str for the caller's convenience.

\$\endgroup\$
  • \$\begingroup\$ Thanks, i feel awful for not ending up which some code like yours ;-; \$\endgroup\$ – Edenia Oct 17 '14 at 18:43
  • \$\begingroup\$ There's nothing to feel awful about. You had a function that worked. Now you have a function that works better. I encourage you to keep practicing and posting. \$\endgroup\$ – 200_success Oct 17 '14 at 18:45
  • \$\begingroup\$ I encourage myself to do that too. By the way isn't it supposed to be memmove(str + from, str + to, 1 + strlen(str + to)+1); \$\endgroup\$ – Edenia Oct 17 '14 at 18:46
  • \$\begingroup\$ On that way it seems won't crop one letter if i call like splice_away(str, 2, 2); \$\endgroup\$ – Edenia Oct 17 '14 at 18:48
  • \$\begingroup\$ 1 + strlen(str + to)+1 would be the same as 2 + strlen(str + to), wouldn't it? I want to copy as many bytes as there are in the substring, plus 1 byte for the terminator. +1 is correct; +2 is too many. \$\endgroup\$ – 200_success Oct 17 '14 at 18:50
8
\$\begingroup\$

A few notes:

  • I think the basic concept of what you are trying to do is remove a substring from a string. My confusion comes in as to why we have a function prototype such as yours rather than this:

    void removeSubstring(char *str, const char *toRemove)
    

    This in my opinion is easier to operate and understand from the caller's standpoint, and will get the same job done. In fact, there are more pros from doing this method, since you don't have to perform as many checks on the values passed to from and to (which you haven't seemed to implement, what if the to value is less than from?!).

  • With this new function prototype, we can now more easily take advantage of the function strstr(str1, str2), which returns a pointer to the first occurrence in str1 of the entire sequence of characters specified in str2, or a NULL pointer if the sequence is not present in str1.

    while((str = strstr(str, toRemove)))
    
  • Now we can use some memmove() magic to remove the substring from our original string.

    size_t length = strlen(toRemove);
    memmove(str, str + length, 1 + strlen(str + length));
    

    No allocation for a new string (always a plus)!

  • You should always declare parameters as const if the function doesn't intend to change its value.

  • You should be using size_t instead of a long.

  • Your i variable has no purpose in your method, as well as your commented out line of code. Both should be removed.


Final implementation:

void removeSubstring(char *str, const char *toRemove)
{
    size_t length = strlen(toRemove);
    while((str = strstr(str, toRemove)))
    {
        memmove(str, str + length, 1 + strlen(str + length));
    }
}

int main(void)
{
    char* test = strdup("test this out");
    removeSubstring(test, " this");
    printf("%s\n", test);
}
test out
Program ended with exit code: 0
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.