动态可阻阵列

Question

如何为动态和可重新大小的数组数据结构改进此代码？

Array.h

#ifndef _ARRAY_H_
#define _ARRAY_H_

class Array
{
private:
    int * m_ArrayContainer;//These two variables
    int m_Size;            //work in pairs.

private:
    void SetSize(int size);     
    void AllocateMemoryOfSize(int size);
    void DeallocateMemory();        

public:
    Array();
    Array(int size);
    Array(Array & arr);
    void operator = (Array & arr);      
    void Resize(int newSize);
    int GetSize();
    void SetItem(int index, int value);
    int GetItem(int index);
    ~Array();
};

#endif

Array.cpp

#include "Array.h"
#include <iostream>

#pragma region Private Methods

void Array :: SetSize(int size)
{
    this->m_Size = size;
}
void Array :: AllocateMemoryOfSize(int size)
{
    this->m_ArrayContainer = new int[size];
    this->SetSize(size);
}
void Array :: DeallocateMemory()
{           
    if(this->GetSize() > 0)
    {
        delete [] this->m_ArrayContainer;
        this->SetSize(0);       
    }
}   
void Array :: SetItem(int index, int value)
{
    this->m_ArrayContainer[index] = value;
}
#pragma endregion

Array :: Array()
{
    //std::cout<<"Array()...\n";

    this->m_ArrayContainer = NULL;

    this->SetSize(0);
}
Array :: Array(int size)
{
    //std::cout<<"Array(int size)...\n";

    this->SetSize(0);

    this->Resize(size);
}
void Array :: Resize(int newSize)
{
    int oldSize = this->GetSize();

    for(int i=0 ; i<oldSize ; i++)
    {
        this->SetItem(i, NULL);
    }
    this->DeallocateMemory();

    this->AllocateMemoryOfSize(newSize);
    this->SetSize(newSize);
}
Array :: Array(Array & arr)
{
    //std::cout<<"Array(Array & arr)...\n";

    this->SetSize(0);

    int size = arr.GetSize();       

    this->Resize(size);

    for(int i=0 ; i<size ; i++)
    {
        this->SetItem(i, arr.GetItem(i));
    }
}
void Array :: operator = (Array & arr)
{
    //std::cout<<"operator = (Array & arr)...\n";

    this->SetSize(0);

    int size = arr.GetSize();       

    this->Resize(size);

    for(int i=0 ; i<size ; i++)
    {
        this->SetItem(i, arr.GetItem(i));
    }
}       
int Array :: GetSize()
{
    return this->m_Size;
}   
int Array :: GetItem(int index)
{
    return this->m_ArrayContainer[index];
}
Array :: ~Array()
{       
    //std::cout<<"~Array()...\n";
    this->DeallocateMemory();       
}

Answer 1

移动语义

因为这是2015年，你应该使用C++14。

您可能应该实现和运算符。

停止使用this->

在C++中，这被认为是不好的实践。如果您必须使用它来消除标识符的歧义，那么您在命名标识符方面做得很糟糕(这是一种代码气味)。

你真的需要一个固定的尺寸吗？

void Array :: SetSize(int size)

看上去有点迟钝。因为这是一个私有成员，这是一个私有方法。你并没有为自己买任何额外的间接层。

删除在nullptr

上工作

您可以在nullptr上进行delete。在调用delete之前，不需要检查空/nullptr。就一直这么做。这确保您总是释放已分配的资源。

void Array :: DeallocateMemory()
{           
    if(this->GetSize() > 0)
    {
        delete [] this->m_ArrayContainer;
        this->SetSize(0);       
    }
}   

看operator[]

void Array :: SetItem(int index, int value)
{
    this->m_ArrayContainer[index] = value;
}
int Array :: GetItem(int index)
{
    return this->m_ArrayContainer[index];
}

设置/获取都没问题。但它不允许您修改适当的值。因此，使用operator[]更为标准，它将返回对内部存储对象的引用，从而允许您修改相应的对象。另外，如果您的数组在某个点被展开以容纳其他类型。然后，您将花费时间将数据复制到和输出Array中。通过提供引用，您可以避免复制操作(如果您不需要它)。

更喜欢初始化程序列表.

应该初始化构造函数中的所有成员。当你这样做的时候。您应该更喜欢使用初始化程序列表。

Array :: Array()
{
    //std::cout<<"Array()...\n";

    this->m_ArrayContainer = NULL;

    this->SetSize(0);
}

// I would have done this:
Array::Array()
    : m_ArrayContainer(new int[0])
    , m_Size(0)
{}

在构造函数的这个版本中。您忘记初始化m_ArrayContainer成员。

Array :: Array(int size)
{
    //std::cout<<"Array(int size)...\n";

    this->SetSize(0);

    this->Resize(size);
}

这意味着成员具有一个不确定的值。从未定义变量中读取将导致“未定义行为”。您必须在读取值之前设置该值。所以你应该在构造函数中初始化它，除非你有一个很好的理由。

将旧项设置为空没有任何意义。

你在挖整数。将这些值设置为NULL没有任何意义。NULL用于表示指针。如果您对NULL使用了正确的值，即nullptr，编译器就会给出一个错误。这样做的原因是NULL宏将自动转换为值为0的整数。

void Array :: Resize(int newSize)
{
    int oldSize = this->GetSize();

    for(int i=0 ; i<oldSize ; i++)
    {
        this->SetItem(i, NULL);
    }
    this->DeallocateMemory();

    this->AllocateMemoryOfSize(newSize);
    this->SetSize(newSize);
}

也是在重新调整尺寸的时候。你不应该先做交易。如果新内存分配失败，则对象处于无效状态，无法修复损坏。

另外，您不希望将数据从原始数组复制到新数组中！这通常是调整大小所做的事情。如果没有将值复制到新的数据存储区域，那么这很可能被称为Realloc。

这些程序应是：

  1) Allocate new space.
     If this fails throw an exception.
  2) Copy the data from the old data storage to the new data storage.
     If this fails (it should not fail for integers but can fail for other types).
     First deallocate storage from step (1) then throw an exception.
  3) Swap the data arrays (swaps are exception safe).
  4) Deallocate the space of the original storage area.

复制构造函数.

通常，您传递被const&复制的东西。您通常不需要修改源代码，因此为了确保您不会意外地将传递的内容更改为常量，这将确保编译器获得意外的更改。

你又忘了把所有的记忆器都设置为初始状态。

Array :: Array(Array & arr)
{
    //std::cout<<"Array(Array & arr)...\n";

    this->SetSize(0);

    int size = arr.GetSize();       

    this->Resize(size);

    for(int i=0 ; i<size ; i++)
    {
        this->SetItem(i, arr.GetItem(i));
    }
}

for()循环可以用几种更好的方式编写。

复制和交换成语

在这里，在您知道一个副本将工作之前，您将销毁旧的数据。因此，有可能使对象处于无效状态。查找副本并交换成语。这是一种安全有效的执行任务的方式。

void Array :: operator = (Array & arr)
{
    //std::cout<<"operator = (Array & arr)...\n";

    this->SetSize(0);

    int size = arr.GetSize();       

    this->Resize(size);

    for(int i=0 ; i<size ; i++)
    {
        this->SetItem(i, arr.GetItem(i));
    }
}       

Const正确性

如果方法正在检索数据，但不更改对象。然后，您应该将函数标记为const。因此，如果将对象作为const对象传递，则仍然可以调用标记为const的方法。

int GetSize();

// This method does not change the state of the object.
// It should thus be declared const.

int GetSize() const;

在一个简单示例中的上述所有点：

class Array
{
private:
    int  Size;
    int* data;

public:
    Array(int size = 0)        // Have a default value and this works
        : size(size)           // as a normal and default constructor.
        , data(new int[size])
    {}
    ~Array()
    {
        delete [] data;
    }
    Array(Array const& copy)    // Copy constructor
        : size(copy.size)
        , data(new int[size])
    {
        std::copy(copy.data, copy.data + size, data);
    }
    Array& operator=(Array const& copy) // Assignment operator
    {
        Arr tmp(copy);         // Make your copy (do it safely into a temp variable). If it fails this variable is not affected.
        tmp.swap(*this);       // Now swap the temp and this variable so
        return *this;          // this is now a copy of the input.
    }                          // temp variable destoryed here. Thus releasing
                               // the original value.
    // The easiest way to move.
    // Is to just swap the current object with the
    // object you are moving. This way the src object
    // is guaranteed to be in a valid state (so its destructor will work)
    Arr(Arr&& move)            noexcept // Move constructor.
        : size(0)
        , data(nullptr)
    {
        move.swap(*this);
    }
    Arr& operator(Arr&& move)  noexcept // Move assignment.
    {
        move.swap(*this);
        return *this;
    }
    void swap(Arr& other)      noexcept // swap (is no exception)
    {
        std::swap(data, other.data);
        std::swap(size, other.size);
    }
    void resize(int newSize)
    {
        Arr tmp(newSize);
        std::copy(data, data + std::min(size, newSize), tmp.data);
        tmp.swap(*this);
    }
    int getSize() const                    {return size;}
    int&       operator[](int index)       {return data[index];}
    int const& operator[](int index) const {return data[index];}
};
void swap(Arr& lhs, Arr& rhs)
{
    lhs.swap(rhs);
}

Answer 2

你有几处内存泄漏。

首先，只有当对象的大小大于0时，DeallocateMemory()才会删除对象的内存块：

void Array :: DeallocateMemory()
{           
    if(this->GetSize() > 0)  // GetSize() will return zero at this point
    {
        delete [] this->m_ArrayContainer;
        this->SetSize(0);       
    }
}   

但是，在C++中分配0大小的内存块是合法的。(有关更多信息，请参见Q&A C++新int[0] --它会分配内存吗？ on 堆栈溢出。)当您分配内存时，您实际上得到了两件事:所请求的内存和C++运行时用于跟踪该内存块的一些数据。

如果您的代码序列如下：

Array a(0);
a.Resize(1);

构造函数将为C++运行时分配一个0大小的内存块和额外的数据。当Resize()调用DeallocateMemory()时，它将看到数据块大小为0，不会调用delete[]，并在AllocateMemoryOfSize()覆盖对象的m_ArrayContainer指针时泄漏内存：

void Array :: AllocateMemoryOfSize(int size)
{
    this->m_ArrayContainer = new int[size]; // Memory leak!
    this->SetSize(size);
}

其次，在赋值操作符中，首先要做的是将this的大小设置为零：

void Array :: operator = (Array & arr)
{
    this->SetSize(0);
    int size = arr.GetSize();       
    this->Resize(size);
    /// [stuff elided].
}       

接下来，使用其他对象的大小调用Resize：

void Array :: Resize(int newSize)
{
    int oldSize = this->GetSize();

    for(int i=0 ; i<oldSize ; i++)
    {
        this->SetItem(i, NULL);
    }
    this->DeallocateMemory();

    this->AllocateMemoryOfSize(newSize);
    this->SetSize(newSize);
}

请注意，开始时的for-loop实际上不会做任何事情，因为到这里时，this的大小已经设置为零。即使没有，它也是没有必要的，因为它的目的是在不久之后对内存进行delete[]，并且一旦它被返回到堆中，内存的内容就无关紧要了。

然后调用DeallocateMemory()，它不会删除任何内存，因为this中的size字段已经设置为零。最后，调用AllocateMemoryOfSize()，它无条件地覆盖指针到数组，泄漏内存的方式与前面的第一点相同。

为了修复这些bug，我会：

1. 检查m_ArrayContainer是否为空指针，以决定是否需要delete[]。void :：DeallocateMemory() { if(this->m_ArrayContainer != NULL) { delete [] this->m_ArrayContainer；this->m_ArrayContainer = NULL；this->SetSize(0)；}}
2. 在删除内存之前，不要在this中将内存清零；然后，您的Resize()方法变成: void :：Resize(int newSize) { this->DeallocateMemory()；this->AllocateMemoryOfSize(newSize)；this->SetSize(newSize)；}
3. 不要独立于内存的分配或取消分配来设置大小字段。在代码中有几处调用SetSize()：删除除AllocateMemoryOfSize()和DeallocateMemory()中的调用之外的所有调用。在赋值操作符的上下文中，这将变成: void :：operator = (Array & arr) { int size = arr.GetSize()；this->Resize(size)；for(int i=0；iSetItem(i，arr.GetItem(I)；}}

请注意，这个类还有其他可能失败的方式，例如，如果在分配期间抛出异常。我建议阅读有关异常保证的内容:简单地说，首先执行内存分配并将其他实例复制到临时内存中，然后如果成功，则释放此实例中的资源并将临时值分配给它们。Scott在他的书“有效C++”中对此做了很好的介绍。

说到赋值操作符，通常会让它返回对this的引用：

Array &Array::operator= (Array & arr)
{
    // [rest of the assignment operator elided]
    return *this;
}

这将使您能够将以下任务链接在一起：

Array a, b, c;
// ... stuff ...
a = b = c;

Answer 3

以下是几点评论：

1. 我会添加更多的注释来精确地指定每个函数的功能。例如，我认为调用Resize只会复制现有的元素，但它会删除现有的项。
2. GetItem和SetItem中有一个bug --如果我用一个太大的index调用，那么我们就会遇到一个异常。您应该更改该函数，说它可以抛出异常，或者至少可以更好地处理index。
3. 还可以更改SetItem，因为它是一个可调整大小的数组，以便复制现有元素，并重新分配内部数组，使其大小与输入index相同，然后只需在那里设置value。
4. 您包括了<iostream>，但是它的所有用途都被注释掉了。
5. #pragma region Private Methods是否用于调试？