2015-12-18 179 views
6

C++で動的文字列配列を作成しようとしています。コンソールに私の動的な文字列配列の内容を表示しようとすると、私はこのエラーが表示されます。ここでは例外エラー:アクセス違反の読み取り場所0xDDDDDDDD

Exception thrown at 0x0FD670B6 (msvcp140d.dll) in Assignment4.exe: 0xC0000005: Access violation reading location 0xDDDDDDDD. 

は私のコードです:

DynamicStringArray.h

#pragma once 
#include "stdafx.h" 
#include <string> 
#include <iostream> 

using namespace std; 

class DynamicStringArray 
{ 
public: 
    DynamicStringArray(); 
    DynamicStringArray(DynamicStringArray &array); 
    ~DynamicStringArray(); 
    int getSize(); 
    void displayContents(); 
    void addEntry(const string &addElement); 
    string getEntry(int index); 
    int deleteEntry(const string &deleteElement); 

private: 
    string *dynamicArray; 
    int size; 
}; 

DynamicStringArray.cpp

#include "stdafx.h" 
#include "DynamicStringArray.h" 
#include <string> 
#include <iostream> 

using namespace std; 

DynamicStringArray::DynamicStringArray() 
{ 
    dynamicArray = NULL; 
    size = 0; 
} 

DynamicStringArray::DynamicStringArray(DynamicStringArray &array) 
{ 
    if (dynamicArray != NULL) 
    { 
     size = 0; 
     delete [] dynamicArray; 
     dynamicArray = NULL; 
    } 

    size = array.getSize(); 
    dynamicArray = new string[size]; 
    for (int i = 0; i < size; i++) 
     dynamicArray[i] = array.dynamicArray[i]; 
} 

DynamicStringArray::~DynamicStringArray() 
{ 
    cout << "In destructor." << endl; 
    delete [] dynamicArray; 
    dynamicArray = NULL; 
} 

int DynamicStringArray::getSize() 
{ 
    return size; 
} 

void DynamicStringArray::displayContents() 
{ 
    if (size != 0) 
     for (int i = 0; i < size; i++) 
      cout << "Item-" << i << ": " << dynamicArray[i] << endl; 
    else 
     cout << "Array is empty." << endl; 
} 

void DynamicStringArray::addEntry(const string &addElement) 
{ 
    string *temp = new string[size + 1]; 
    for (int i = 0; i < size; i++) 
      temp[i] = dynamicArray[i]; 
    temp[size] = addElement; 
    size++; 
    delete [] dynamicArray; 
    dynamicArray = temp; 
    delete[] temp; 
} 

string DynamicStringArray::getEntry(int index) 
{ 
    if ((index >= 0) && (index < size)) 
    { 
     return dynamicArray[index]; 
    } 
    return NULL; 
} 

int DynamicStringArray::deleteEntry(const string &deleteElement) 
{ 
    if(size == 0) 
    { 
     return false; 
    } 

    for (int i = 0; i < size; i++) 
    { 
     if (dynamicArray[i] == deleteElement) 
     { 
      string *temp = new string[size - 1]; 
      for (int x = 0; x < size - 1; ++x) 
      { 
       if (x < i) 
        temp[x] = dynamicArray[x]; 
       else 
        temp[x] = dynamicArray[x + 1]; 
      } 

      delete[] dynamicArray; 
      dynamicArray = temp; 
      delete[] temp; 
      --size; 
      return true; 
     } 
    } 

    return false; 
} 

メイン:

int main() 
{ 
    DynamicStringArray dsArray1; 
    cout << "dsArray1.displayContents():" << endl; 
    dsArray1.displayContents(); // Should indicate array is empty 
    cout << "Display dsArray1.getSize()= " << dsArray1.getSize() << endl; 

    dsArray1.addEntry("Entry-A"); 
    dsArray1.displayContents(); 

    dsArray1.addEntry("Entry-B"); 
    dsArray1.displayContents(); 
    dsArray1.addEntry("Entry-C"); 
    dsArray1.displayContents(); 
    return 0; 
} 

私が間違っていることを誰にでも教えてもらえますか?どうすればこの問題を解決できますか?

+2

単に 'std :: vector 'を使わなかったのはなぜですか? – PaulMcKenzie

答えて

5

このすべてはすでに std::vector<std::string>を使用して利用可能です。 std::vectorクラスはC++が提供する動的配列クラスであり、使用できるものの自家製バージョンを作る理由はほとんどない。


これは明らかに、コピーコンストラクタが間違っているということです。 dynamicArrayは初期化されていないですが、あなたはここでそれを使用します。

if (dynamicArray != NULL)

dynamicArrayが持っているどのような値保証はありません。この修正は、コピーコンストラクタでコードのこのブロック全体を削除することです:コピーコンストラクタは、新しいオブジェクトを作成し

if (dynamicArray != NULL) 
{ 
    size = 0; 
    delete [] dynamicArray; 
    dynamicArray = NULL; 
} 

ので、そこにNULLポインタのための「プレテスト」の理由にはならないので、不必要な作業を行います。オブジェクトは存在しなかったことを覚えていますので、予備的なことはありません。


第二の問題は、あなたがaddEntrydeleteEntry機能でdelete [] temp;コールを発行しているということです。 dynamicArrayに割り当てたメモリの割り当てを解除するので、これらの行を削除してください。


第3の問題は、ユーザー定義の代入演算子が不足していることです。代入演算子は次のシグネチャがあり、そしてあなたは、実装を提供する必要があります:両方のオブジェクトがスコープ外に行くときに別のDynamicStringArrayからDynamicStringArrayは、メモリリークやメモリの二重解放の原因となります割り当て、この機能がないと

DynamicStringArray& operator=(const DynamicStringArray&); 

を。

一つの実装はcopy/swap idiomを使用することができます。

#include <algorithm> 
//... 
DynamicStringArray& DynamicStringArray::operator=(const DynamicStringArray& rhs) 
{ 
    DynamicStringArray temp(rhs); 
    std::swap(temp.dynamicArray, dynamicArray); 
    std::swap(temp.size, size); 
    return *this; 
} 

もう一つの問題はこれです:

string DynamicStringArray::getEntry(int index) 
{ 
    if ((index >= 0) && (index < size)) 
    { 
     return dynamicArray[index]; 
    } 
    return NULL; // <-- Undefined behavior if this is done 
} 

NULLでstd::stringオブジェクトを割り当てることが未定義の動作です。空の文字列を返すか、インデックスが範囲外の場合は例外をスローします。結論として


、私は非常にそれが正しいコピー・セマンティクスを実装する必要がありますクラスを設計に来るとき、あなたがRule of 3上に読むことをお勧めします。

+2

'addEntry'の代入はすべて' std :: string'sなので、これはうまくいきます( 'operator ='の実装はまだまだ良い考えです)。最後の行 'delete [] temp; 'にある問題 –

+0

@MilesBudnek' addEntry'と 'deleteEntry'の両方で問題を指摘する答えを更新しました。 – PaulMcKenzie

関連する問題