2015-02-13 2 views
0

Я хочу инициализировать структуры в цикле и обновить массив. Следующий код, который я использую.Инициализация нескольких структур в функции cpp

#include <iostream> 

struct Record { 
    char *name; 
}; 

struct Response { 
    Record *records[]; 
}; 


int main() 
{ 
    Response *response = new Response; 

    for(int i=0; i<4; i++) { 

     Record *record= new Record(); 

     char x[20];sprintf(x, "%d", i); 

     record->name = (char*)x; 

     response->records[i] = record; 

     std::cout << "Inserting: " << x << "\n"; 

     //Not sure if I have to delete. 
     delete record; 
    } 

    for(int i=0; i<4; i++) { 
     std::cout << "Fetching: " << response->records[i]->name << "\n"; 
    } 
} 

Странно при печати всех элементов в массиве печатать одинаковое значение. Любая идея, комментарий или мысли о том, что здесь может быть неправильным?

Пример вывода:

Inserting: 0 
Inserting: 1 
Inserting: 2 
Inserting: 3 
Fetching: 3 
Fetching: 3 
Fetching: 
Fetching: 3 
+5

Вам действительно не нужно выполнять все выделение/выделение ручной памяти в C++. Ваш код не использует преимущества языка C++ или стандартной библиотеки. – juanchopanza

+3

Если вы не указали указатели, ваши проблемы волшебным образом исчезнут. – chris

+1

Массив 'records' в структуре' Response' не поддерживается, что означает, что любой индекс в нем будет за пределами границ. –

ответ

5

Вы хранения указателей на локальные переменные в объектах, доступ к которым после их удаления.

Даже если Response::records были надлежащим образом рассчитаны, эти два фактора сделают код недействительным.

Идиоматические C++ будет использовать std::string и std::vector<Record>, но если вы действительно хотите строки C-стиля и массивов, это должно работать:

struct Record { 
    char name[20]; 
}; 

struct Response { 
    Record records[4]; 
}; 

int main() 
{ 
    Response response; 

    for(int i = 0; i < 4; i++) { 
     Record record; 
     sprintf(record.name, "%d", i); 
     response.records[i] = record; 
     std::cout << "Inserting: " << record.name << "\n"; 
    } 

    for(int i = 0; i < 4; i++) { 
     std::cout << "Fetching: " << response.records[i].name << "\n"; 
    } 
} 
3

Самая большая проблема в том, что это C не C++, однако, давайте рассмотрим решение C:

есть несколько проблем:

struct Response { 
    Record *records[]; 
}; 

не забудьте дать измерение для массива:

Record *records[4]; 

Следующие:

struct Record { 
    char *name; 
}; 

Это просто объявляет указатель с именем именем, нет памяти, выделенной для имени в точку. Вам нужно будет выделить память для имени в будущем или сделать ее статическим массивом здесь вместо указателя.

record->name = (char*)x; 

Теперь имя указывает на статический массив x. Это будет происходить каждый раз через цикл, поэтому все экземпляры record-> name будут указывать на один и тот же массив. Вот почему все они печатают одно и то же значение :-) Вам нужно скопировать строку в x в имя записи->, которую вы не можете сделать, потому что нет никакого хранилища, связанного с именем record->.

В 2 пути выхода являются:

struct Record { 
    char name[20]; // static array 
}; 
... 
char x[20] = {0}; // initialise so we're sure of a terminating null 
strcpy(record->name, x); 

Или

struct Record { 
    char *name; 
}; 
... 
record->name = new char[20]; 
strcpy(record->name, x); 

Наконец,

//Not sure if I have to delete. 
delete record; 

Нет, вы не должны удалять здесь, так как эти записи будут использоваться впоследствии , Если вы должны удалить, настройте что-то вроде цикла инициализации, но только до конца main(), только на этот раз вы будете уничтожать. Конечно, если это C, вы не должны использовать новые и удалять. malloc() и бесплатное() семейство вызовов лучше подходят для стиля C. Основным преимуществом новых и удаления над malloc() и free() является вызов конструкторов и деструкторов выделенных объектов в нужное время, но вы не использовали ни одну из этих функций.

Это было бы намного проще, короче и безопаснее на C++. Должны ли вы делать это на C или вы считаете C++?

#include <iostream> 
#include <cstring> 

struct Record { 
    char *name; 
}; 

struct Response { 
    Record *records[4]; 
}; 


int main() 
{ 
    Response *response = new Response; 

    for(int i=0; i<4; i++) { 

     Record *record= new Record(); 

     char x[20];sprintf(x, "%d", i); 

     record->name = new char[20]; 
     strcpy(record->name, x); 

     response->records[i] = record; 

     std::cout << "Inserting: " << x << "\n"; 

     //Not sure if I have to delete. 
     //delete record; 
    } 

    for(int i=0; i<4; i++) { 
     std::cout << "Fetching: " << response->records[i]->name << "\n"; 
    } 


    // the program is about to exit and the resources will be freed by the system, so this is not strictly necessary 
    for(int i=0; i<4; i++) { 
     delete [] response->records[i]->name; 
     delete response->records[i]; 
    } 

    delete response; 
} 

EDIT: Вот один из возможных решений, что нет более С ++ дружественными, не сырых указателей, все распределение памяти осуществляются с помощью стандартной библиотеки в строке и векторе.

#include <iostream> 
#include <string> 
#include <vector> 

using namespace std; 

struct Record { 
    Record(string record_name) : name (record_name) {} 
    string name; 
}; 

struct Response { 
    vector<Record> records; 
}; 

int main() 
{ 
    Response response; 
    for(int i=0; i<4; i++) { 
     response.records.push_back(Record(to_string(i))); 
     std::cout << "Inserting: " << i << "\n"; 
    } 

    for (auto r : response.records) { 
     cout << "Fetching: " << r.name << "\n"; 
    } 

} 
+0

'record_name' должно быть передано ссылкой const. Вы должны использовать '[const] auto &' или вы делаете много копий. –

Смежные вопросы