2015-05-19 3 views
-3

Может кто-нибудь объяснить, почему этот код не работает? Он продолжает сбой, когда он запрашивает ввод в addCar().Добавление элемента в массив struct C++

Я думаю, что что-то не так с копированием массива, но я не могу понять, что именно. Я также пытался использовать copy(), но он тоже не работал.

#include <iostream> 
#include <string> 
using namespace std; 
/* run this program using the console pauser or add your own getch, system("pause") or input loop */ 


struct Car{  
     string Brand; 
     string model; 
     long mileage; 
}; 

void addCar(int *ptr, struct Car *arra){ 
     *ptr=*ptr+1; 

     Car *newArr = new Car[*ptr]; 

     memcpy(newArr, arra, (*ptr)*sizeof(Car)); 

     cout<<"Brand "; 
     getline(cin,newArr[*ptr].Brand); 

     cout<<"Model "; 
     getline(cin, newArr[*ptr].model); 

     cout<<"mileage "; 
     cin>>newArr[*ptr].mileage; 

     arra=newArr;  
}; 

int main(int argc, char** argv) { 

     int size=1; 
     int *ptr_size; 
     ptr_size=&size; 

     Car *tab=new Car[*ptr_size]; 

     tab[0].Brand = "Audi"; 
     tab[0].model = "A8"; 
     tab[0].mileage = 14366; 

     addCar(*ptr_size, tab); 

     return 0;  
} 
+0

Что вы пытаетесь сделать? Что делает код неправильно? – rlbond

+1

Вы передаете целое число в первый аргумент 'addCar', который хочет указатель. – stackptr

+1

'memcpy' не является безопасным способом копирования строки. – IronMensan

ответ

0

Нерабочее является возможно здесь:

getline(cin,newArr[*ptr].Brand); 

Немного выше, вы сделали это: *ptr=*ptr+1; и сделал newArr массив из *ptr элементов. Массивы являются нулевыми. Это означает, что первый элемент массива - newArr[0]. Последнее будет на newArr[*ptr-1], поэтому запись в newArr[*ptr] записывает чужую память. Как правило, это плохо.

Но это тоже не круто:

*ptr=*ptr+1; 

Car *newArr = new Car[size+1]; 

memcpy(newArr, arra, (*ptr)*sizeof(Car)); 

Вы увеличиваете размер массива. Ничего страшного.

Вы создаете новый массив с новым размером. Ничего страшного.

Вы копируете новый размер элементов из старого массива в новый массив и забрасываете конец старого массива. Не в порядке.

Лучший ответ предоставлен Джерри Коффином и Полом Маккензи в комментариях: используйте std::vector. Если это не разрешено ... Ick.

Но ладно тогда.

Во-первых, memcpy буквально копирует блок памяти. Он не знает и не заботится о том, что такое блок памяти или что он содержит. Никогда не используйте memcpy, если вы не копируете что-то действительно, очень простое, например, базовый тип данных или структуру, состоящую только из основных типов данных. Строка не является базовой. Данные, представленные строкой, могут быть не внутри строки. В этом случае вы копируете указатель на строку и этот указатель не будет действителен после смерти строки. Это не проблема в вашем случае, потому что вы не убиваете строку. Это приводит к проблеме 2. Давайте исправим это, прежде чем вы туда доберетесь. Самый простой способ (кроме vector) будет:

for (int index = 0; index < *ptr-1; index++) 
{ 
    newArr[index] = arra[index]; 
} 

Оптимизация примечание. Вы не хотите изменять размер и копировать массив каждый раз, когда вы его добавляете. Рассмотрим наличие двух целых чисел, одного размера массива и другого индекса в массиве и удвоения размера массива каждый раз, когда индекс собирается догнать размер.

Когда вы выделяете любую память для данных с помощью new, кто-то должен очистить и вернуть эту память с помощью delete. В C++, что вы кто-то.так что перед вами arra=newArr; вам необходимо delete[] arra;

Передача в индексе массива как указателя чрезмерно сложна. Используйте ссылку или просто перейдите по значению и верните новый индекс. Кроме того, не называйте переменную ptr. Используйте что-то описательное.

void addCar(int &arrasize, struct Car *arra){

или

int addCar(int arrasize, struct Car *arra){

Следующая проблема: int addCar(int arrasize, struct Car *arra){ проходит в указатель на ARRA. Но вы передали указатель по значению, сделали копию указателя, поэтому, когда вы меняете указатель внутри функции, это только копия, которая была изменена, и новый массив больше не будет возвращаться. Так,

int addCar(int arrasize, struct Car * & arra){

передает ссылку на указатель и позволяет изменить указатель внутри функции.

Собираем все вместе:

int addCar(int size, struct Car * & arra) 
{ 
    Car *newArr = new Car[size + 1]; 

    for (int index = 0; index < size; index++) 
    { 
     newArr[index] = arra[index]; 
    } 

    cout << "Brand "; 
    getline(cin, newArr[size].Brand); 

    cout << "Model "; 
    getline(cin, newArr[size].model); 

    cout << "mileage "; 
    cin >> newArr[size].mileage; 

    delete[] arra; 
    arra = newArr; 
    return size+1; 
} 

int main() 
{ 
    int size=1; 

    Car *tab=new Car[size]; 

    tab[0].Brand = "Audi"; 
    tab[0].model = "A8"; 
    tab[0].mileage = 14366; 

    size = addCar(size, tab); 

    // do more stuff; 
    // bit of test code here 
    for (int index = 0; index < size; index++) 
    { 
     cout << "Car " << index << " brand =" <<tab[index].Brand << " Model=" << tab[index].model << " mileage=" <<tab[index].mileage << endl; 
    } 
    delete[] tab; 
    return 0; 
} 
0

При копировании старого массива в новый вы обращаетесь недействительных памяти, помните, что в тот момент, ARRA имеет размер * PTR-1 не * PTR, поэтому линия должна быть

 memcpy(newArr, arra, (*ptr-1)*sizeof(Car)); 

также в других линиях, вы должны ввести новое значение в * PTR-1 положении, так как индексы в newArr идут от 0 до размера-1 т * PTR-1:

cout<<"Brand "; 
    getline(cin,newArr[*ptr-1].Brand); 

    cout<<"Model "; 
    getline(cin, newArr[*ptr-1].model); 

    cout<<"mileage "; 
    cin>>newArr[*ptr-1].mileage;