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

Playing with strings

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

Problem

I wanted to dust off my C skills, so I decided to write a couple string manipulation functions.
I aimed for simplicity and safety. Performance was not a concern.

Please be a thorough as you like. C is not my everyday language, so I might have made
some obvious mistakes. I would also appreciate extra attention to the test cases I've provided.
Are they easy to understand and expand? Is coverage good enough?

``
#include
#include
#include
#include

// Erases a part of the string's content, filling the end with null chars.
//
startIndex must be in a valid range inside this string. The char pointed by startIndex is included.
// If
count is less than 0 or greater than the string's length, erases until the end, starting at startIndex.
// Does nothing if the string is empty or if
count is zero.
size_t strErase(char * str, size_t len, size_t startIndex, int count)
{
assert(str != NULL);
if (*str == '\0')
{
assert(len == 0); // This would indicate a parameter mismatch if not true.
return len;
}

// If the string is not empty, a length must be provided.
assert(len != 0);
assert(startIndex = len)
{
memset(&str[startIndex], 0, len - startIndex);
return len - startIndex; // New length.
}

// Erase
count chars from an arbitrary position:
size_t charsToMove = len - (startIndex + count) + 1; // Including the null terminator (+1)
memmove(&str[startIndex], &str[startIndex + count], charsToMove);
return len - count; // New length.
}

// Replaces a sequence of characters in a string by a single character.
//
startIndex must be in a valid range inside this string. The char pointed by startIndex is included.
// If
count is less than 0 or greater than the string's length, fills with replaceWith char starting at startIndex.
// Does nothing if the string is empty or if
count` is zero.
void strReplace(char * str, size_t len, size_t startIndex, int count, char replaceWith)
{
ass

Solution

-
Asserts are good in the test cases, but not in the actual code. First, in a release version they are no-ops, so the release doesn't check anything. Second, they result in a program termination, which is a bit drastic.

-
Interfaces are unclear. What is len? It may only represent the length of the character array (vs C string), i.e. you do not rely on a terminating 0, and this, in turn, annuls the claim that strErase is filling the end with null chars. If you do rely on a terminating 0, do not have len as a parameter, but calculate it.

-
Treating negative count is a somewhat unorthodox. Normally (if allowed at all) it means "count backwards", that is, erase a [startIndex + count, startIndex) range.

Context

StackExchange Code Review Q#73783, answer score: 4

Revisions (0)

No revisions yet.