2015-12-18 20 views
6

Sto provando a creare una matrice di stringhe dinamica in C++. Quando si tenta di visualizzare il contenuto della mia matrice di stringhe dinamica alla console ricevo questo errore:Errore di eccezione: posizione di lettura della violazione di accesso 0xDDDDDDDD

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

Ecco il mio codice:

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; 
} 

principale:

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; 
} 

Qualcuno può dirmi cosa sto facendo male. Come posso risolvere questo problema?

+2

Qualsiasi motivo per cui non si è semplicemente utilizzato 'std :: vector '? – PaulMcKenzie

risposta

5

Si noti che tutto questo è già disponibile utilizzando std::vector<std::string>. La classe std::vector è la classe dinamica dell'array fornita da C++ e non vi è alcun motivo per realizzare versioni fatte in casa di ciò che è disponibile.


Detto questo, un problema evidente è che il costruttore di copie non è corretto. Il dynamicArray non è inizializzato, ma lo si utilizza qui:

if (dynamicArray != NULL)

v'è alcuna garanzia che valore ha dynamicArray. La correzione è quello di rimuovere l'intero blocco di codice nel costruttore di copia:

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

Dal momento che il costruttore di copia costruisce un nuovo oggetto, non v'è alcun motivo di "pre-test" per un puntatore NULL e quindi fare lavori inutili. Ricorda che l'oggetto non esisteva, quindi non c'è nulla di preliminare da fare.


Il secondo problema è che si sta l'emissione di una chiamata delete [] temp; nelle addEntry e deleteEntry funzioni. Rimuovi queste righe, poiché stai deallocando la memoria che hai appena assegnato a dynamicArray.


Il terzo problema è che ti manca l'operatore di assegnazione definita dall'utente. L'operatore di assegnazione ha la seguente firma, ed è necessario fornire l'attuazione:

DynamicStringArray& operator=(const DynamicStringArray&); 

Senza questa funzione, assegnando un DynamicStringArray ad un altro DynamicStringArray causerà perdite di memoria e doppia deallocazione di memoria quando entrambi gli oggetti escono di portata.

Un'implementazione può utilizzare il 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; 
} 

Un altro problema è questo:

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

È comportamento indefinito di assegnare un oggetto std::string con NULL. Restituisce una stringa vuota o genera un'eccezione se l'indice è fuori limite.


In conclusione, mi raccomando di leggere sul Rule of 3 quando si tratta di progettare classi che devono implementare la semantica di copia corretta.

+2

I compiti in 'addEntry' sono tutti' std :: string's, quindi vanno bene (implementare un 'operator =' sarebbe comunque una buona idea). Il problema c'è con l'ultima riga 'delete [] temp;' –

+0

@MilesBudnek Ho aggiornato la risposta per segnalare il problema sia con 'addEntry' che' deleteEntry'. – PaulMcKenzie