Я не хочу быть злым, но у этого кода достаточно проблем, трудно решить, с каких из них начинать.
using namespace std;
Вот первый красный флаг.Все, что я могу сказать, это то, что using namespace std;
плохая идея.С другими пространствами имен это может быть приемлемо, но с std
его всегда следует избегать (IMO).
struct Intersection
{
unsigned short road;
long long int *roadId;
short *lane[2];
};
Это выглядит как довольно плохо спроектированная структура.std::vector
это хорошая вещь.Используй это.Вместо того, чтобы просто представлять структуру данных, вы можете определить operator>>
для своего типа структуры, чтобы вы могли читать его напрямую.Даже если вы этого не сделаете, то, как вы это используете, на самом деле вам нужно что-то вроде:
struct road {
long long Id;
short lane[2];
};
struct Intersection {
int road_count;
road *roads;
};
Тогда вместо пары параллельных массивов одинакового размера, которыенужно идти параллельно, вы получите несколько дорог, каждая со своими данными.std::vector
все же лучше.
int main(int argc, char** argv)
{
std::ifstream file;
cout<<"file: "<<argv[1]<<endl;
file.open(argv[1], std::ios::in);
Вместо того, чтобы определять объект ifstream и открывать его отдельно, вы должны обычно планировать передачу имени ктору, чтобы оно определялось иоткрывается в одной операции, что-то вроде:
std::ifstream file(argv[1]);
Но вы также обычно хотите добавить немного проверки ошибок, поэтому вы пытаетесь использовать аргумент командной строки в качестве имени файла, если он был переданПримерно так:
if (argc < 2) {
std::cerr << "Usage: your_command <filename>\n";
return EXIT_FAILURE;
}
Тогда у вас будет код для определения ifstream
.
while (!file.eof())
Вот еще одна серьезная проблема.Цикл этой формы по сути всегда неправильный (включая этот случай, судя по всему).
cout<<"Start Intersection"<<endl;
Intersection *a = new Intersection;
Кажется, нет причин выделять это динамически.Вы, возможно, выздоравливающий (или, возможно, не восстанавливающийся) программист на Java или C #?Java требует, чтобы все объекты пользовательских классов были размещены динамически, а C ++ - нет.
file>>a->road;
a->roadId = new long long int[a->road];
a->lane[0] = new short[a->road];
a->lane[1] = new short[a->road];
cout<<"a->road: "<<a->road<<endl;
for (int i=0; i<a->road; i++)
{
file>>a->roadId[i];
cout<<endl<<"a->roadId["<<i<<"]: "<<a->roadId[i]<<endl;
file>>a->lane[i][0];
cout<<"a->lane["<<i<<"][0]: "<<a->lane[i][0]<<endl;
file>>a->lane[i][1];
cout<<"a->lane["<<i<<"][1]: "<<a->lane[i][1]<<endl;
}
Я бы предпочел отделить код для чтения данных от кода для отображения данных.За исключением таких вещей, как домашняя работа (или отладка), вы редко хотите отображать много необработанных данных во время чтения.В любом случае код чтения должен обычно находиться в operator>>
для этого класса, а код отображения в operator<<
для класса.
cout<<"Intersection inserted"<<endl;
Это, похоже, прямая ложь.Вы на самом деле не вставили Intersection
во что-либо.
delete a;
Когда вы прекратите выделять Intersection
динамически, вы также сможете устранить это.Если вы настаиваете на обработке всего динамического выделения вручную, вам необходимо удалить компоненты до этого, чтобы избежать утечек памяти (еще одна причина, чтобы предпочесть std::vector
).
Я знаю, что это может звучать довольно негативно,и это оставляет меня немного разорванным.С одной стороны, я действительно хотел бы , чтобы предложить лучшие способы сделать что-то.В то же время, это выглядит как домашнее задание, что я крайне не решаюсь просто опубликовать лучший код.Я попытался включить несколько подсказок о лучших способах, но понимаю, что они, вероятно, не настолько конкретны, как вы хотели бы - я извиняюсь за это, но, учитывая, что это, вероятно, домашняя работа, я не думаю, что могунамного конкретнее.