【问题标题】:Flaws in algorithm and algorithm performance算法和算法性能的缺陷
【发布时间】:2009-07-08 05:55:43
【问题描述】:
char *stringmult(int n)
{
    char *x = "hello ";
    for (int i=0; i<n; ++i)
    {
        char *y = new char[strlen(x) * 2];
        strcpy(y,x);
        strcat(y,x);
        delete[] x;
        x=y;
    }
    return x;
}

我正试图找出这部分的缺陷是什么。一方面,它删除 x,然后尝试将其值复制到 y。另一个是 y 是 x 的两倍,并且 y 永远不会被删除。有什么我想念的吗?而且,我需要弄清楚如何获得算法性能。如果您有一个快速链接来学习如何操作,我将不胜感激。

【问题讨论】:

  • 请贴出完整代码。
  • 甜蜜的僵尸耶稣是邪恶的代码。
  • @Earwicker:这使它成为'C with Classes'。我根本不会称它为 C++。
  • @rlbond:在 Java 中将所有内容放在 static void main(String[]) 下不会变成 C;像 C 一样使用 C++,不要让它变得更少 C++。
  • 这个邪恶的代码对于求职面试非常有用。太棒了。

标签: c++ string algorithm memory-management


【解决方案1】:

y 需要比strlen(x) * 2 多一个字节来为终止的 nul 字符腾出空间——仅供初学者使用。

无论如何,当您返回 newed 内存区域时,由调用者来删除它 (eek)。

在我看来,你缺少的是std::string...!-)

至于性能,用strcpy复制N个字符是O(N);将 N1 个字符连接到具有 N2 前一个 strlen 的 char 数组是 O(N1+N2) (std::string 更快,因为它将字符串的长度保持在 O(1) 可访问属性中!-)。因此,只需将 N+N**2 求和,直到您感兴趣的极限是多少(如果您想要的只是一个大 O 估计值,您可以忽略 N+ 部分,因为它显然会随着越来越大的值而下降N!-)。

【讨论】:

  • 更“eek”;当使用 n=0 调用时,调用者不应删除它。 :-)
【解决方案2】:

对于初学者 delete[] x;第一次在一些静态内存上循环运行。不好。

这看起来像是试图返回一个包含字符串“hello”的 2^n 个副本的缓冲区。所以最快的方法是计算出副本的数量,然后为整个结果分配一个足够大的缓冲区,然后用内容填充它并返回它。

void repeat_string(const std::string &str, int count, std::vector<char> &result)
{
    result.resize(str.size() * count);
    for (int n = 0; n < count; n++)
        str.copy(&result[n * s.size()], s.size());
}

void foo(int power, std::vector<char> &result)
{
    repeat_string("hello ", 1 << (power + 1), result); 
}

【讨论】:

    【解决方案3】:
    1. 无需在循环中调用 strlen() - 只需调用一次;
    2. 当调用 new 时,不会为空字符请求空格 - 将导致未定义的行为;
    3. 应该使用 strcpy 而不是 strcat - 您已经知道在哪里复制第二个字符串并通过 strcat 查找字符串的结尾需要额外的计算;
    4. delete[] 用于静态分配的字符串文字 - 会导致未定义的行为;
    5. 虽然您提前知道结果长度,但内存会不断重新分配 - 内存重新分配非常昂贵

    您应该立即计算结果长度并立即分配内存并将 char* 作为参数传递:

    char* stringMult(const char* what, int n)
    {
         const size_t sourceLen = strlen( what );
         int i;
         size_t resultLen = sourceLen;
         // this computation can be done more cleverly and faster
         for( i = 0; i < n; i++ ) {
            resultLen *= 2;
         }
         const int numberOfCopies = resultLen / sourceLen;
         char* result = new char[resultLen + 1];
         char* whereToWrite = result;
         for( i = 0; i < numberOfCopies; i++ ) {
            strcpy( whereToWrite, what );
            whereToWrite += sourceLen;
         }
         return result;
    }
    

    我的实现的某些部分可以优化,但仍然要好得多,并且(我希望)不包含未定义行为类错误。

    【讨论】:

      【解决方案4】:

      您必须在为 Y 分配空间以用于 NULL 终止字符串时添加一个 检查以下位置的代码http://codepad.org/tkGhuUDn

      【讨论】:

      • hmmm.. char * target = malloc(source_len*m+1) * sizeof(char));
      【解决方案5】:
      char * stringmult (int n)
      {
          int i;
          size_t m;
          for (i = 0, m = 1; i < n; ++i; m *= 2);
          char * source = "hello ";
          int source_len = strlen(source);
          char * target = malloc(source_len*m+1) * sizeof(char));
          char * tmp = target;
          for (i = 0; i < m; ++i) {
              strcpy(tmp, source);
              tmp += source_len;
          }
          *tmp = '\0';
          return target;
      }
      

      这里是纯 C 的更好版本。您的代码的大部分缺点都已消除,即删除未分配的指针,strlennew的使用过多>。 尽管如此,我的版本可能意味着与您的版本相同的内存泄漏,因为调用者负责随后释放字符串。

      编辑:更正了我的代码,感谢Sharptooth。

      【讨论】:

      • 这将复制字符串 n 次,并且 OP 的代码复制它 2^n 次。
      • 而且你调用 strlen 和 strcpy 的频率太高了——这可以更快地完成。
      • 除了你应该使用 size_t 作为“元素数量”变量之外更好。否则此代码不可移植。
      • 没错,你只需要调用 strcpy n 次,但是底层的复制需要同样的时间,因为字符串会变得更大。如果需要进一步优化代码,则取决于调用 strcpy 的成本。
      【解决方案6】:

      char* string_mult(int n)

      {

      const char* x = "hello ";
      
      char* y;
      
          int i;
      
      
      
      for (i = 0; i < n; i++)
      
      {
      
          if ( i == 0)
      
          {
      
              y = (char*) malloc(strlen(x)*sizeof(char));
      
              strcpy(y, x);
      
          }
      
          else
      
          {
      
              y = (char*)realloc(y, strlen(x)*(i+1));
      
              strcat(y, x);
      
          }
      
      }
      
      return y;
      

      }

      【讨论】:

      • 这伤害了我的眼睛——把特殊情况放在循环之外,而是从 1 运行到 n :)
      【解决方案7】:

      没有人会指出“y”实际上正在被删除?

      甚至没有提到Schlmeiel the Painter

      但我对这个算法做的第一件事是:

      int l = strlen(x);
      int log2l = 0;
      int log2n = 0;
      int ncopy = n;
      while (log2l++, l >>= 1);
      while (log2n++, n >>= 1);
      if (log2l+log2n >= 8*(sizeof(void*)-1)) {
          cout << "don't even bother trying, you'll run out of virtual memory first";
      }
      

      【讨论】:

      • 嗯...在哪里? x 被删除,然后 y 被复制到 x 中。
      • @sharptooth:没错,所以在下一次迭代中猜猜 x 是什么......宾果游戏!来自上一次迭代的 y!不用费心说那最后会有泄漏,因为那是被返回的内存。
      • 不,在下一次迭代中,y 将附加到一个新分配的内存块上,然后 x 将被删除,y 将被复制到 x 中。这东西很好用。在调用者忘记释放内存之前不会发生泄漏。
      • 不,什么? x 和 y 是指针,所以 y = new...; x = y;删除 x;有效地删除为 y 分配的内存。我看不出你的意思。
      • 如果 x=y 在删除之前完成,就会出现这种情况,但在 OP 的代码中,首先删除完成,然后 x=y。
      猜你喜欢
      • 2020-06-06
      • 2011-01-16
      • 1970-01-01
      • 2018-07-14
      • 1970-01-01
      • 2016-12-07
      • 2015-02-16
      • 2013-08-07
      • 1970-01-01
      相关资源
      最近更新 更多