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

Inserting a parenthesized nickname

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

Problem

Given two strings, for example


s1 = "Valentino Rossi" (which may contain as many spaces between the 2 words as I like)

s2 = "Vale"

I have to create a function that requests these two strings and that it outputs another one like this one: "Valentino (Vale) Rossi".

So for example if I have


s1 = "Mario Belli"

s2 = "Mar"

the returned string must be "Mario (Mar) Belli".

#include 
#include 
#define STR_LEN 32

char *makeNickname(char *s1, char *s2, char *s3);

int main() {
    char nomeCognome[] = "Corrado     Righetti"; 
    char nickname[] = "Cora";
    char nameNickname[STR_LEN];
    makeNickname(nomeCognome, nickname, nameNickname);
    printf("%s\n\n", nameNickname);
}

char *makeNickname(char *s1, char *s2, char *s3) {
    char *p = s1, *q = s2, *k = s3;
    while (*p) {
        if (*p != ' ') {
            *k++ = *p++;
        } else {
            *k++ = *p;
            *k++ = '(';
            while (*q) {
                *k++ = *q;
                q++;
            }
            *k++ = ')';
            *k++ = *p;
            while (*p == ' ') p++;
        }       
    }
    return s3;
}

Solution

Naming

Your variable names are polyglot, cryptic, and inconsistent. Don't mix Italian with English — stick with English if you can. Instead of s1s3, pick more meaningful names. Instead of p, q, and k, it would have been better to use p1, p2, and p3 — but again, more meaningful names would be even better.

Function signatures

In modern C, you should mark strings as const if you're not going to write to them.

It is customary to put main() at the end, so that you don't need to declare your functions.

Approach

You aren't writing a '\0' terminator character to out, so your function won't work if the buffer is unclean.

Your solution is not robust to unexpected input, such as extra spaces at the end of s1.

Character-by-character manipulation isn't very readable. You wouldn't want to write code like this in a larger project. Instead, make use of the existing library functions for working with strings. (You have already written #include , so why not?)

char *makeNickname(const char *names, const char *nickname, char *out) {
    const char *firstSpace = strchr(names, ' ');
    const char *lastName = firstSpace + strspn(firstSpace, " ");
    sprintf(out, "%.*s (%s) %s", (int)(firstSpace - names), names, nickname, lastName);
    return out;
}


Note that this solution, like yours, doesn't check for buffer overflow. It would be good practice to also pass the size of the buffer to enable such a check.

Code Snippets

char *makeNickname(const char *names, const char *nickname, char *out) {
    const char *firstSpace = strchr(names, ' ');
    const char *lastName = firstSpace + strspn(firstSpace, " ");
    sprintf(out, "%.*s (%s) %s", (int)(firstSpace - names), names, nickname, lastName);
    return out;
}

Context

StackExchange Code Review Q#111749, answer score: 4

Revisions (0)

No revisions yet.