我的连接函数 mystrcat(char*, char*, char*) 有什么问题？

Question

我最近接受了采访，要求写mystrcat(*s1, *s2, *s3)，其中s1 和s2 是源字符串，连接结果由s3 给出。有人告诉我，不要担心s3 的内存分配，并假设s1 和s2 不是空/无效字符串。所以我写了以下蹩脚（粗鲁）的程序。我被告知s3 有问题，或者s3 可能出现问题。你能告诉它是什么/可能是什么吗？

void mystrcat(char *s1, char *s2, char *s3)
{
    if (! (s1 || s2 || s3)) return; // one/more pointers are invalid

    // copy string s1 into s3
    while(*s1) {
      *s3 = *s1;
      s1++;
      s3++;
    }

    // concatenate string s2 into s3
    while(*s2) {
      *s3 = *s2;
      s2++;
      s3++;
    }

    *s3 = '\0';
}

你能告诉我这里有什么问题吗？有什么更专业的做法？

Answer 1

两个可能的点

首先，您被告知输入和输出指向有效字符串，因此可以说不需要有效性测试。如果需要它，你应该大声失败。更好的是：

 void mystrcat(char *s1, char *s2, char *s3)
    {
        ASSERT( s1 );
        ASSERT( s2 );
        ASSERT( s3 );
        ....

然后，您基本上可以在可以重用它们时编写 strcat/strcpy：

void mystrcat(char *s1, char *s2, char *s3)
{
    strcpy( s3, s1 );
    strcat( s3, s2 );
}

如果我面试你不是为了一个初级职位，我希望你向我指出指定的 mystrcat 界面设计得非常好，并详细说明了你将如何改进它。

Answer 2

if (! (s1 || s2 || s3) return; // one/more pointers are invalid

应该是

if ((!s1) || (!s2) || (!s3)) return;

Answer 3

这将是我的 cmets

• s1 和 s2 都应该输入到const char*，因为您无意修改它们。
• 您没有验证 s3 是否有足够的分配空间来容纳 s1 和 s2 的组合长度
• 当用户传入错误数据（NULL 值）时，您正在静默失败。这不好，因为调用者无法区分成功和失败的调用
• 您验证 s1、s2 和 s3 不为空的方法不正确（Igor 是对的）

我希望你在面试期间提出或自己回答的问题

• 我应该如何表达复制字符串失败？
• 您想要 strcat 的精确克隆，还是更现代且参数验证更好的版本？
• 我可以使用其他标准的 str 函数来完成答案吗？

Answer 4

除了前面的答案，还有一件事可能会出错： s3 可以指向 s1 或 s2 字符串的中间。如果这是一个合法的条件，那么你需要更复杂的实现。

Answer 5

您的函数定义可能会受到一些批评，但在问题陈述的约束下，您的函数将产生正确的答案。这在技术上并没有错。

我的猜测是问题没有得到有效的沟通，或者面试官的批评没有得到有效的沟通。也许澄清这些是测试的一部分，嗯？

这里是可能的投诉的快速摘要...

• 此行有逻辑错误...

  如果 (!(s1 || s2 || s3)) 返回;

...因为如果 all 为空，它将返回，但如果 any 为空，您可能希望返回。这不会导致失败，因为问题陈述说没有一个可以为空。

• 上面提到的同一行静默失败。最好返回错误代码、抛出异常或断言。静默失败是不好的做法，但在技术上仍然是正确的，因为考虑到问题的领域，该功能不会失败。
• s1 和 s2 可以是 const char* 类型，以提高可读性和效率（const 有助于某些编译器优化）。
• 修改参数变量通常被认为是不好的 实践。另一个问题 可读性。
• 您可以使用现有函数 strcpy 和 strcat。另一件事 可读性和效率。

一个伟大的程序员应该在可读性、效率和强大的错误处理方面超越技术正确性，但这些大多是主观的和情境性的。无论如何，不​​管你的第一个 if 语句中的错误，我认为你的函数读得很好并且可以完成工作。 :)

Answer 6

1. 无效检查

   如果 (! (s1 || s2 || s3)) 返回; // 一个/多个指针无效

   它实际上检查是否至少有一个指针不为 0，例如至少一个指针是有效的。 应该是

   如果 (!s1 || !s2 || !s3) 返回;

2. 你没有检查 s3 是否很大 够了，或者如果你在外面写 （但假设 s3 很大 够了吧？ - 但它仍然会 在现实世界中不起作用）

3. 您未能跳过 null 从 s1 的末尾复制 进入s3。您在 s3 中的 0 之后附加 s2 所以它会结束 "s1stringvalueNULLs2stringvalue" （此处的 NULL 表示值 0 或 null 或 在实际代码中为零，这是为了 插图）。任何 C 方法 期望空终止的字符串将 只看到“s1stringvaluepart”和 将在 null 处停止。

这里有一个解决方案：

void mystrcat(char *s1, char *s2, char *s3)
{
    if (!s1 || !s2 || !s3) return;

    while (*s3++ = *s1++);
    if (!*(s3-1)) --s3;             // reverse over null in s1
    while (*s3++ = *s2++);

    *s3 = 0;
}

您可以使用以下方法对其进行测试：

#include <iostream>
using namespace std;

int main()
{
    char s1[] = "short";
    char s2[] = "longer";
    char s3[20];

    memset(s3, 0, sizeof(s3));
    mystrcat(0,s2,s3);
    cout << "2: " << s3 << endl;

    memset(s3, 0, sizeof(s3));
    mystrcat(s1,0,s3);
    cout << "3: " << s3 << endl;

    memset(s3, 0, sizeof(s3));
    mystrcat(s1,s2,0);
    cout << "4: " << s3 << endl;

    memset(s3, 0, sizeof(s3));
    mystrcat(s1,s2,s3);
    cout << "1: " << s3 << endl;

}

Answer 7

如果我错了，请纠正我。复制s1后s3的最后一个字符不会是'\0'吗？那么在某些情况下，s2 中的剩余字符永远无法读取？

据我了解

while(*s3++ = *s1++);

将复制字符直到达到 NULL 并且 '\0' 不是 NULL，或者它是否被视为这样？如果我可以假设 s1 = "Hello" 和 s2 = "world!"然后在复制 s1 之后，s3 看起来像： Hello\0 然后复制“世界！” bit 会导致 s3 看起来像：Hello\0 world!\0

这有意义吗？我的理解正确吗？