HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

Trim a string to a given start and end

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
trimstartandendstringgiven

Problem

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);
}

Solution

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.

Code Snippets

void strdev (char *str, long from, long to)
char *part1 = malloc(strlen(str) - strlen(&str[from]));
memmove(part1, str, from); // New copying
sprintf(str, "%s%s", part1, &str[to+1]);
char *splice_away(char *str, int from, int to)
{
    memmove(str + from, str + to, 1 + strlen(str + to));
    return str;
}

Context

StackExchange Code Review Q#67018, answer score: 10

Revisions (0)

No revisions yet.