我无法得到我想要的正确输出，而且每次都会改变答案

Posted 2023-02-21

【中文标题】我无法得到我想要的正确输出，而且每次都会改变答案

【英文标题】：I can't get the right output that I want and the answer changes every time

【发布时间】：2020-11-18 22:17:06

【问题描述】：

所以我正在尝试为这个问题编写代码：

是的，我必须使用数组，因为这是必需的。

考虑将两个 n 位二进制整数相加的问题，存储在两个 n 元素数组 A 和 B 中。两个整数的和应以二进制形式存储在 (n+1) 元素数组 C 中。正式陈述问题并编写两个整数相加的伪代码。

我知道ans 数组在addd 函数的末尾包含正确的输出。但是，我无法输出该答案。

下面是我的代码。请帮我弄清楚我在哪里出错了，以及我可以做些什么来改变它以使其正常工作。我将不胜感激。

#include <iostream>
using namespace std;

int * addd(int a[], int n1, int b[], int n2)

    int s;
    if(n1<n2) s=n2+1;
    else s=n1+1;
    int ans[s];
    int i=n1-1, j=n2-1, k=s-1;
    int carry=0;
    while(i>=0 && j>=0 && k>0)
    
        ans[k]=(a[i]+b[j]+carry)%2;
        //cout<<k<<" "<<ans[k]<<endl;
        carry=(a[i]+b[j]+carry)/2;
        i--; j--; k--;
    
    //cout<<"Carry "<<carry<<endl;
    ans[0]=carry;
    return ans;


int main(int argc, const char * argv[]) 
    // insert code here...
    int a[]=0,0,0,1,1,1;
    int n1=sizeof(a)/sizeof(a[0]);
    int b[]=1,0,1,1,0,1;
    int n2=sizeof(b)/sizeof(b[0]);
    int *p=addd(a,6,b,6);
//    cout<<p[1]<<endl;
//    cout<<p[0]<<" "<<p[1]<<" "<<p[2]<<" "<<p[3]<<" "<<p[4]<<" "<<p[5]<<" "<<p[6]<<endl;
    return 0;

【问题讨论】：

您可以查看“代码审查”堆栈交换。

【参考方案1】：

using namespace std;

不要写using namespace std;。当我在 Code Review Stack Exchange 中处于活动状态时，我从一个常见问题的文件中粘贴了一个摘要，但这里没有。相反，你应该只声明你需要的符号，比如using std::cout;

int * addd(int a[], int n1, int b[], int n2)

int a[] 形式的参数很奇怪。这来自 C，实际上被转换为 int* a 并且本身并没有传递数组。

输入应该是const。

名称不清楚，但我猜n1 是数组的大小？在标准指南中，您会看到强烈建议不要传递指针加长度。标准指南库提供了一个简单的 span 类型来代替它。

长度应该是size_t 而不是int。

根据描述，我认为每个元素只有一位，对吧？那么为什么int 类型的数组呢？我会使用 bool 或 int8_t 更容易使用。

你要返回什么？如果a 和b 及其长度是输入，那么您要返回指向开头的指针的输出在哪里？这没有给出值语义，因为您正在返回一个指向必须存在于其他地方的东西的指针，那么它的生命周期是多少？

    int s;
    int ans[s];

    return ans;

嗯，这就是你的问题。首先，声明一个大小不是常量的数组甚至是不合法的。 （这是一个 gnu 扩展，它实现了 C 的 VLA 功能，但并非没有问题，因为它破坏了 C++ 类型系统） 不管怎样，你返回的是一个指向本地数组第一个元素的指针，那么当函数返回时内存会发生什么？轰隆隆。

    int s;

没有。在创建值时对其进行初始化。

    if(n1<n2) s=n2+1;
    else s=n1+1;

了解图书馆。 怎么样：

const size_t s = 1+std::max(n1,n2);

然后获取记忆的便携方式是：

std::vector<int> ans(s);

如果一个数组比另一个数组短，您的主要逻辑将不起作用。较短的输入应该表现得好像它有前导零来匹配。考虑抽象“获取下一位”的问题，这样您就不会重复处理每个输入的代码并造成难以理解的混乱。你真的应该先学会使用集合和迭代器。

现在：

    return ans;

会按预期工作，因为它是一个值。您只需要将函数声明为正确的类型。所以只需使用auto 作为返回类型，它就知道了。

    int n1=sizeof(a)/sizeof(a[0]);

没有哦。

有一个standard function 来给出内置原始数组的大小。但实际上，这应该作为传递的一部分自动完成，而不是像前面提到的那样作为单独的事情。

    int *p=addd(a,6,b,6);

你写了6 而不是n1 等等。 无论如何，通过以前的编辑，它变成：

using std::size;
const auto p = addd (a, size(a), b, size(b));

最后，关于：

   cout<<p[0]<<" "<<p[1]<<" "<<p[2]<<" "<<p[3]<<" "<<p[4]<<" "<<p[5]<<" "<<p[6]<<endl;

使用循环怎么样？

for (auto val : p)  cout << val;
cout << '\n';

哦，不要使用endl。无论如何，自动刷新的 cout 都不需要它，而且它很慢。现代最佳实践是在需要时使用 '\n' 然后显式使用 flush（例如，从不）。

【讨论】：

非常感谢您抽出时间输入答案

【参考方案2】：

我们来看看：

 int ans[s];

除了这甚至不是标准的一部分而且可能编译器会给你一些警告（参见link），该命令在堆栈中分配临时内存，在函数退出时被释放：这就是为什么你得到每个时间不同的结果，您正在读取垃圾，即同时可能已被覆盖的内存。 例如，您可以将其替换为

 int* ans = new int[s];

不要忘记在使用完缓冲区后（函数外）释放内存，以避免内存泄漏。

---

其他一些注意事项：

    int s;
    if(n1<n2) s=n2+1;
    else s=n1+1;

这可以更优雅地写成：

    const int s = (n1 < n2) ? n2 + 1 : n1 + 1;

另外，实际的计算代码是不精确的，因为如果 n1 不等于 n2 会导致错误的结果：您需要进一步的代码来完成最长数组的剩余位的处理。顺便说一句，由于您定义 s 的方式，您不需要检查 k > 0。 以下应该有效：

    int i=n1-1, j=n2-1, k=s-1;
    int carry=0;
    while(i>=0 && j>=0)
    
        ans[k]=(a[i]+b[j]+carry)%2;
        carry=(a[i]+b[j]+carry)/2;
        i--; j--; k--;
    
    while(i>=0) 
        ans[k]=(a[i]+carry)%2;
        carry=(a[i]+carry)/2;
        i--; k--;
    
    while(j>=0) 
        ans[k]=(b[j]+carry)%2;
        carry=(b[j]+carry)/2;
        j--; k--;
    
    ans[0]=carry;
    return ans;

【讨论】：

非常感谢您的回复！！我实际上是在等待这段代码的输出工作，以便在两个数组的长度不同时处理实例。您使用 int* ans = new int[s]; 的答案就是我要找的东西，非常感谢！

使用new其实是一个很差的设计。 Core Guidelines 反复告诫，“No Naked New”。此逻辑代码重复相同的块 3 次，这是很差的。 “写得更优雅……”是真的，但忽略了已经存在的标准算法。

@JDlugosz 问题陈述需要使用分配的缓冲区。正确的做法是使用 std::vector。其余的，如果您指的是 std::max，它恰好在某些平台上完全低效

我认为std::max 会使用任何仔细的定相和/或编译器提示来确保生成最佳代码；例如有条件的移动指令。直接写同样的逻辑如何做得更好？

【参考方案3】：

如果你只能使用 C 数组

返回ans 是返回指向局部变量的指针。 then 函数返回后，指针所指的对象不再有效，因此尝试读取它会导致未定义的行为。

解决此问题的一种方法是将地址传递给一个数组以保存您的答案，然后填充它，而不是使用 VLA（这是一种非标准 C++ 扩展）。

VLA（可变长度数组）是一个从运行时计算值中获取大小的数组。在你的情况下：

int s;
//... code that initializes s
int ans[s];

ans 是 VLA，因为您没有使用常量来确定数组大小。但是，这不是 C++ 语言的标准特性（它是 C 语言中的可选特性）。

你可以修改你的函数，让ans实际上是由调用者提供的。

int * addd(int a[], int n1, int b[], int n2, int ans[])

    //...

然后调用者将负责传入一个足够大的数组来保存答案。

您的功能似乎也不完整。

    while(i>=0 && j>=0 && k>0)
    
        ans[k]=(a[i]+b[j]+carry)%2;
        //cout<<k<<" "<<ans[k]<<endl;
        carry=(a[i]+b[j]+carry)/2;
        i--; j--; k--;
    

如果一个数组比另一个数组短，那么较短数组的索引将首先到达0。然后，当相应的索引变为负数时，循环将停止，而不处理较长数组中的剩余项。这实质上使ans 中的相应条目未初始化。读取这些值会导致未定义的行为。

为了解决这个问题，您应该使用基于carry 的正确计算来填充ans 中的剩余条目以及较长数组中的剩余条目。

---

更多的 C++ 方法

上面的原始答案是假设您被限制为只能使用 C 样式数组进行输入和输出，并且您想要一个可以让您与原始实现保持接近的答案。

下面是一个更面向 C++ 的解决方案，假设您仍然需要提供 C 数组作为输入，但除此之外没有其他约束。

C 数组包装器

C 数组不提供您在使用 C++ 容器时可能习惯拥有的便利。要获得其中一些不错的特性，您可以编写一个适配器，让 C 数组的行为类似于 C++ 容器。

template <typename T, std::size_t N>
struct c_array_ref 
    typedef T ARR_TYPE[N];
    ARR_TYPE &arr_;

    typedef T * iterator;
    typedef std::reverse_iterator<T *> reverse_iterator;

    c_array_ref (T (&arr)[N]) : arr_(arr) 

    std::size_t size ()  return N; 

    T & operator [] (int i)  return arr_[i]; 
    operator ARR_TYPE & ()  return arr_; 

    iterator begin ()  return &arr_[0]; 
    iterator end ()  return begin() + N; 

    reverse_iterator rbegin ()  return reverse_iterator(end()); 
    reverse_iterator rend ()  return reverse_iterator(begin()); 
;

使用 C 数组引用

您可以通过引用传入数组，并使用模板参数推导来推导数组大小，而不是传入两个参数作为有关数组的信息。

返回std::array

虽然您无法像在问题中尝试的那样返回本地 C 数组，但您可以返回包装在 struct 或 class 中的数组。这正是便利容器std::array 所提供的。当您使用 C 数组引用和模板参数推导来获取数组大小时，您现在可以在编译时计算 std::array 应具有的返回值的正确数组大小。

template <std::size_t N1, std::size_t N2>
std::array<int, ((N1 < N2) ? N2 : N1) + 1>
addd(int (&a)[N1], int (&b)[N2])

规范化输入

如果您假设参数已按特定顺序排列，则解决问题会容易得多。如果你总是希望第二个参数是更大的数组，你可以通过一个简单的递归调用来做到这一点。这是非常安全的，因为我们知道递归最多会发生一次。

    if (N2 < N1) return addd(b, a);

使用 C++ 容器（或相似适配器）

我们现在可以将我们的参数转换为前面显示的适配器，还可以创建一个std::array 来保存输出。

    c_array_ref<int, N1> aa(a);
    c_array_ref<int, N2> bb(b);
    std::array<int, std::max(N1, N2)+1> ans;

尽可能利用现有算法

为了解决您的原始程序的缺点，您可以稍微调整您的实现以尝试消除特殊情况。一种方法是存储将较长数组添加到0 的结果并将其存储到输出中。但是，这主要可以通过简单地调用std::copy 来完成。

    ans[0] = 0;
    std::copy(bb.begin(), bb.end(), ans.begin() + 1);

由于我们知道输入仅包含 1s 和 0s，因此我们可以计算从较短数组到较长数组的直接加法，而无需考虑进位（将在下一步中解决）。为了计算这个加法，我们将std::transform 与 lambda 表达式一起应用。

    std::transform(aa.rbegin(), aa.rend(), ans.rbegin(),
                   ans.rbegin(),
                   [](int a, int b) -> int  return a + b; );

最后，我们可以通过输出数组来修复进位计算。这样做之后，我们就可以返回结果了。返回是可能的，因为我们使用std::array 来表示答案。

    for (auto i = ans.rbegin(); i != ans.rend()-1; ++i) 
        *(i+1) += *i / 2;
        *i %= 2;
    

    return ans;

更简单的main 函数

我们现在只需要将两个数组传入addd 函数，因为模板类型推导会发现数组的大小。此外，使用ostream_iterator 可以更轻松地处理输出生成器。

int main(int, const char * []) 
    int a[]=1,0,0,0,1,1,1;
    int b[]=1,0,1,1,0,1;
    auto p=addd(a,b);
    
    std::copy(p.begin(), p.end(),
              std::ostream_iterator<int>(std::cout, " "));

    return 0;

Try it online!

【讨论】：

“相反，将地址传递给一个数组来保存你的答案并填充它，而不是使用 VLA（这是一个非标准 C++ 扩展）”是什么意思跨度>

答案已更新。祝你的任务好运！

我喜欢你的解释。您的解释清楚地说明了为什么（基本概念）代码不起作用，对此我非常感激。非常感谢

【参考方案4】：

如果我可以编辑一下...我认为这对于初学者来说是一个看似困难的问题，并且如上所述应该在任何尝试编码之前就在设计审查中指出问题。它告诉您在 C++ 中做一些不好/典型/惯用/不恰当的事情，并用妨碍实际开发逻辑的问题分散您的注意力。

考虑您编写的核心算法（Antonio 已更正）：可以理解和讨论该算法，而无需担心 A 和 B 是如何实际传入以供此代码使用的，或者它到底是哪种集合。如果它们是 std::vector、std::array 或原始 C 数组，则用法相同。同样，如何从代码中返回结果？您在此处填充ans，它如何进入和/或退出代码并返回到main 并不相关。

原始 C 数组不是 C++ 中的一等对象，对于它们如何作为参数传递有特殊规则（从 C 继承）。

返回更糟糕，返回动态大小的东西是 C 语言中的一个主要问题，像这样的内存管理是错误和安全漏洞的主要来源。我们想要的是值语义。

其次，使用数组和下标在 C++ 中不是惯用的。您使用 迭代器 并抽象出集合的确切性质。如果您有兴趣编写本身不处理内存管理的超高效后端代码（它由处理所涉及的实际集合的其他代码调用），它看起来像 std::merge 这是一个古老的函数回到 90 年代初。

template< class InputIt1, class InputIt2, class OutputIt >
OutputIt merge( InputIt1 first1, InputIt1 last1,
                InputIt2 first2, InputIt2 last2,
                OutputIt d_first );

您可以找到具有相似签名的其他人，它们将两个不同的范围用于输入和输出到第三个区域。如果你完全像这样写addp，你可以用硬编码大小的原始C数组调用它：

int8_t A[] 0,0,0,1,1,1;
int8_t B[] 1,0,1,1,0,1;
int8_t C[ ??? ];

using std::begin; std::end;

addp (begin(A),end(A), begin(B), end(B),  begin(C));

请注意，由调用者准备足够大的输出区域，并且没有错误检查。

但是，相同的代码可以用于向量，甚至是不同容器类型的任意组合。这可以通过传递插入迭代器来填充std::vector。但在这个特殊的算法中，这很困难，因为你是在逆序计算它。

std::array

改进原始 C 数组的情况，您可以使用std::array 类，它完全相同的数组，但没有奇怪的传递/返回规则。它实际上只是一个包装结构内的原始 C 数组。请参阅此文档：https://en.cppreference.com/w/cpp/container/array

所以你可以写成：

using BBBNum1 = std::array<int8_t, 6>
BBBNum1 addp (const BBBNum1& A, const BBBNum1& B)  ... 

里面的代码可以和你一样使用A[i]等，但也可以通过A.size()获取大小。这里的问题是输入的长度相同，输出也相同（不是大 1）。使用模板，它可以写成灵活的长度，但仍然只在编译时指定。

std::vector

vector 类似于一个数组，但具有运行时长度。它是动态的，是您应该在 C++ 中使用的首选集合。

using BBBNum2 = std::vector<int8_t>
BBBNum2 addp (const BBBNum2& A, const BBBNum2& B)  ... 

同样，该函数内部的代码可以引用B[j]等，并使用B.size()与array集合完全相同。但是现在，大小是一个运行时属性，并且每个属性都可以不同。

您可以通过将大小作为构造函数参数来创建结果，就像在我的第一篇文章中一样，然后您可以按值返回 vector。请注意，如果您编写，编译器将有效地执行此操作，并且实际上不必复制任何内容：

auto C = addp (A, B);

现在开始真正的工作

好的，既然这种干扰至少已经消失了，您可以担心实际编写实现。我希望您确信使用 vector 代替 C 原始数组不会影响您的问题逻辑，甚至不会影响使用下标的（可用）语法。特别是由于问题涉及伪代码，我将其对“数组”的使用解释为“合适的可索引集合”，而不是具体的原始 C 数组类型。

同时处理 2 个序列并处理不同长度的问题实际上是一个通用的想法。在 C++20 中，Range 库有一些东西可以快速解决这个问题。旧的第 3 方库也存在，您可能会发现它称为 zip 或类似名称。

但是，让我们从头开始编写它。 您想从两个输入中一次读取一个项目，但要巧妙地使其看起来它们的长度相同。你不想写三遍相同的代码，或者详细说明 A 更短或 B 可能更短的情况......只是抽象出它们一起读取的想法，如果一个用完它提供零.

这是它自己的一段代码，可以应用于 A 和 B 两次。

class backwards_bit_reader 
    const BBBnum2& x;
    size_t index;
public:
    backwards_bit_reader(const BBBnum2& x) : xx, indexx.size()  
    bool done() const  return index == 0; 
    int8_t next()
       
       if (done()) return 0;  // keep reading infinite leading zeros
       --index;
       return x[index];
       
 ;

现在您可以编写如下内容：

backwards_bit_reader A_in  A ;
backwards_bit_reader B_in  B ;
while (!A_in.done() && !B_in.done()) 
   const a = A_in.next();
   const b = B_in.next();
   const c = a+b+carry;
   carry = c/2;  // update
   C[--k]= c%2;
 
 C[0]= carry;  // the final bit, one longer than the input

它可以写得更紧凑，但这很清楚。

另一种方法

问题是，写backwards_bit_reader 是否超出了您目前所学的范围？在不重复语句的情况下，您还能如何将相同的逻辑应用于 A 和 B？

您应该学会识别有时称为“代码异味”的东西。多次重复相同的代码块，重复相同的步骤而没有任何改变，但它应用于哪个变量，应该被视为丑陋和不可接受的。

如果它们的长度不同，您至少可以通过确保 B 始终是较长的情况来减少这些情况。如果不是这种情况，请交换 A 和 B，作为初步步骤。 （实际上实现那个井是另一个题外话）

但是逻辑仍然几乎是重复的，因为您必须处理进位一直传播到最后的可能性。刚才你有 2 个副本而不是 3 个。

至少在外观上扩展较短的循环是编写循环的唯一方法。

这个问题有多现实？

它被简化到了愚蠢的地步，但是如果不是在base 2中完成而是具有更大的值，这实际上是在实现多精度算术，这是人们想要做的真实事情。这就是为什么我将BBBNum 上面的类型命名为“Bad Binary Bignum”。

了解实际的内存范围并希望代码快速和优化也是您有时想要做的事情。 BigNum 就是一个例子。您经常在字符串处理中看到这一点。但是我们想要创建一个高效的后端，它在不知道它是如何分配的情况下对内存进行操作，以及调用它的更高级别的包装器。

例如：

void addp (const int8_t* a_begin, const int8_t* a_end,
           const int8_t* b_begin, const int8_t* b_end,
           int8_t* result_begin, int8_t* result_end);

将使用提供的范围进行输出，不知道也不关心它是如何分配的，并采用任何连续范围的输入，而不关心使用什么类型的容器来管理它，只要它是连续的。请注意，正如您在 std::merge 示例中看到的那样，传递 begin 和 end 而不是 begin 和 size 更为惯用。

但是你有如下辅助函数：

BBBNum2 addp (const BBBNum2& A, const BBBNum2& B)

    BBBNum result (1+std::max(A.size(),B.size());
    addp (A.data(), A.data()+A.size(),  B.data(), B.data()+B.size(), C.data(), C.data()+C.size());

现在普通用户可以使用向量和动态创建的结果来调用它，但它仍然可以调用数组、预分配的结果缓冲区等。