这个归并排序算法有什么问题？

如何解决这个归并排序算法有什么问题？

有人能告诉我这段代码有什么问题吗？此代码是使用归并排序对数组中的一组元素进行排序。

#include<iostream>

void merge(int arr[],int left,int mid,int right){
    int left_ptr = left;
    int right_ptr = mid + 1;
    int size = right - left + 1;       
    int temp[size];
    int k = left;

    while (left_ptr <= mid && right_ptr <= right)
    {
        if(arr[left_ptr] <= arr[right_ptr]){
            temp[k] = arr[left_ptr];
            left_ptr++;
            k++;
        }
        else{
            temp[k] = arr[right_ptr];
            right_ptr++;
            k++;
        }
        
    }

    while (left_ptr <= mid)
    {
        temp[k] = arr[left_ptr];
        left_ptr++;
        k++;
    }

    while (right_ptr <= right)
    {
        temp[k] = arr[right_ptr];
        right_ptr++;
        k++;
    }
    
    for (int i = left_ptr; i < k; i++)
    {
        arr[i] = temp[i];
    }
    
}

void mergeSort(int arr[],int right){
    int mid;
    if (left < right)
    {
        mid = (right + left)/2;
        mergeSort(arr,left,mid);
        mergeSort(arr,mid + 1,right);
        merge(arr,mid,right);
    }
    
}
int main(){
    int arr[] = {45,8,9,7,4,58,2,34,58}; 
    std::cout << arr << std::endl; 
    int size = sizeof(arr)/sizeof(int);
    mergeSort(arr,size - 1);
    for (int i = 0; i < size; i++)
    {
        std::cout << arr[i] << "    ";
    }

    std::cout << std::endl;
    
}

我用许多在线代码仔细检查了它，但没有发现任何错误...您认为哪里出了问题？我尝试使用就地数组（类似于快速排序）来实现这一点。

解决方法

这是您的代码有问题的列表。我在这里广义地理解“错误”。对于每一段代码，我的主要批评是基于风格的而不是“正确性”，其中的风格旨在使正确性更容易被发现。

在此过程中，一种风格批评导致发现看起来像错误的东西。

void merge(int arr[],int left,int mid,int right){

您正在使用 int 来引用数组中的偏移量。
您正在使用 int[] 参数，这是 int* arr 的旧 C 语法。改用 std::span 之类的东西。

继续：

 int left_ptr = left;

如果您的目标是保留原始参数并处理副本，请将原始参数设为 const，这样就无需有人证明它们在函数体中没有发生变异。

int right_ptr = mid + 1;

您有名为 _ptr 的变量不是指针。

int size = right - left + 1;

您似乎没有使用半开间隔。使用并学习使用半开音程。它们在 C++ 中是传统的，并且确实摆脱了大量围栏后更正代码。

int temp[size];

这不符合 C++。实际上，即使在支持此功能的编译器上，许多 C++ 实现的堆栈也比您可能想要排序的数组的内存小得多。这会导致您的代码炸毁堆栈。

正确性比性能更重要。在堆栈上创建动态大小的对象会导致程序执行未定义的行为或在其他合理的输入上崩溃。

int k = left;

这个变量没有描述它的作用。

while (left_ptr <= mid && right_ptr <= right)
while (left_ptr <= mid)
while (right_ptr <= right)

这些循环中有很多代码重复。

DRY - 不要重复自己。在这里，如果在任何一个重复中存在错误，如果您进行 DRY，则该错误将被用于所有用途并且更容易被发现。这里有很多 DRY 方法（lambdas、辅助函数、稍微复杂的分支和一个循环）；使用其中之一。

for (int i = left_ptr; i < k; i++)
{
    arr[i] = temp[i];
}

看起来像手动标准副本？看起来它也有一个错误，因为当然手动重新实现 std copy 意味着你做错了。

void mergeSort(int arr[],int right){

再次，传统的 C 风格数组传递。

    int mid;

无需在未初始化的情况下声明它。将声明尽可能靠近它们首次使用的位置，并尽快让它们脱离范围。

    if (left < right)
    {
        mid = (right + left)/2;

使这个int mid =。

        mergeSort(arr,left,mid);
        mergeSort(arr,mid + 1,right);

关闭间隔如何让您不得不进行烦人的围栏张贴的示例。

        merge(arr,mid,right);

mergeSort(arr,size - 1);

这里是另一个围栏 +/- 1。

我在代码中看到两个可能的错误：

在 int temp[size]; 中声明 merge 是无效的，因为 size 不是常量。您将需要动态分配数组。

其次，在 merge 函数的最后一段（for 循环）中，初始化 i = left_ptr。但是，在此之前，left_ptr 被设置为等于 mid。我相信您确实想初始化 i = left。

编辑：刚刚注意到：temp 不一定要从 arr 的开头开始。我的意思是，temp 的每个元素都映射到 arr 的特定元素，但是您的代码假设在几个地方，temp[0] 映射到 arr[0]，这不是真的（temp[0] 实际上映射到 arr[right]）。有两种方法可以解决这个问题。

您可以修复基于此假设的部分。在最后的 arr[i] = temp[i] 循环中使用 arr[i + right] = temp[i] 代替 for，并将 k 初始化为零。

第二个选项是，不是在每次调用 temp 时创建和删除 merge，而是创建它的大小等于 arr 和在算法的整个执行过程中保持它（这可以通过在算法之外创建它并将它传递给每个对 merge 或 MergeSort 的调用来完成）这样，等偏移假设实际上会正确。