-
Segmentation fault
I'm creating a program that reads text files, each with a priority and message, and then prints them in the correct order. Files with higher priority are printed first, and if they have the same priority, the ones that were entered in first are printed first. My code compiles fine, but if more than 3 text files are entered, I get a Segmentation fault. It also has a problem sorting by priority. Is there a way to fix this without significantly changing my code?
Code:
#include <iostream>
#include <fstream>
#include <string>
#include <cassert>
using namespace std;
/* ----------------------- Package Class ----------------------- */
class package
{
public:
int priority;
int order;
string message;
};
/* ----------------------- Main Program ----------------------- */
int main()
{
/* ----------------------- The Variables ----------------------- */
int amount;
package temp;
package *packages;
string fileName;
ifstream fin;
/* ----------------------- Opening the Files ----------------------- */
cout << "\nEnter the amount of packages you want entered: " << endl;
cin >> amount;
packages = new package[amount];
if (packages == 0)
cout << "Memory Cannot Be Allocated";
cout << "\nEnter the names of the packages one by one:" << endl;
for (int i=0; i<amount; i++)
{
cout << "Package " << i+1 << ": ";
cin >> fileName;
fin.open(fileName.data(), ios::in);
assert( fin.is_open() );
fin >> packages[i].priority;
getline(fin, packages[i].message);
packages[i].order = i+1;
fin.close();
}
/* ----------------------- Sorting the Packages ----------------------- */
for (int i=1; i<amount-1; i++)
{
for (int j=0; j=amount-i; j++)
{
if (packages[j].priority > packages[j+1].priority)
{
packages[j] = temp;
packages[j] = packages[j+1];
packages[j+1] = temp;
}
if (packages[j].priority == packages[j+1].priority)
{
if (packages[j].order > packages[j+1].order)
{
packages[j] = temp;
packages[j] = packages[j+1];
packages[j+1] = temp;
}
}
}
}
/* ----------------------- Printing the Files ----------------------- */
cout << "The Packages:";
for (int i=0; i<amount; i++)
{
cout << "\n\n" << "Priority: "<< packages[i].priority << "\nOrder: " << packages[i].order << "\n" << packages[i].message;
}
cout << endl << endl;
delete[] packages;
return 0;
}
Here is an example text file: (the first character is the priority, 1 being highest)
Code:
1This program is awesome
Thanks so much for the help!
-
Code:
packages[j] = temp;
packages[j] = packages[j+1];
packages[j+1] = temp;
Not a very good example of swapping. (I.e., you kill a packages.)
-
yeah, its basically bubble sort...
so what would be a better way?
-
I know you're trying to do bubblesort. I know that you intend those three lines to swap two variables. The problem is, they don't. Trace it out and see.
-
well i thought this change would make it work, but it didn't
Code:
temp = packages[j];
packages[j] = packages[j+1];
packages[j+1] = temp;
-
Did you change both of them? EDIT: Also, this looks suspicious:
Code:
for (int j=0; j=amount-i; j++)
-
i did change both to make temp = packages[j];
and j=amount-i should have been j<amount-i, but still no luck
Here's the updated code:
Code:
/*************
Michael Hickman
ECE 231
*************/
#include <iostream>
#include <fstream>
#include <string>
#include <cassert>
using namespace std;
/* ----------------------- Package Class ----------------------- */
class package
{
public:
int priority;
int order;
string message;
};
/* ----------------------- Main Program ----------------------- */
int main()
{
/* ----------------------- The Variables ----------------------- */
int amount;
package temp;
package *packages;
string fileName;
ifstream fin;
/* ----------------------- Opening the Files ----------------------- */
cout << "\nEnter the amount of packages you want entered: " << endl;
cin >> amount;
packages = new package[amount];
if (packages == 0)
cout << "Memory Cannot Be Allocated";
cout << "\nEnter the names of the packages one by one:" << endl;
for (int i=0; i<amount; i++)
{
cout << "Package " << i+1 << ": ";
cin >> fileName;
fin.open(fileName.data(), ios::in);
assert( fin.is_open() );
fin >> packages[i].priority;
getline(fin, packages[i].message);
packages[i].order = i+1;
fin.close();
}
/* ----------------------- Sorting the Packages ----------------------- */
for (int i=1; i<amount-1; i++)
{
for (int j=0; j<amount-i; j++)
{
if (packages[j].priority > packages[j+1].priority)
{
temp = packages[j];
packages[j] = packages[j+1];
packages[j+1] = temp;
}
if (packages[j].priority == packages[j+1].priority)
{
if (packages[j].order > packages[j+1].order)
{
temp = packages[j];
packages[j] = packages[j+1];
packages[j+1] = temp;
}
}
}
}
/* ----------------------- Printing the Files ----------------------- */
cout << "The Packages:";
for (int i=0; i<amount; i++)
{
cout << "\n\n" << "Priority: "<< packages[i].priority << "\nOrder: " << packages[i].order << "\n" << packages[i].message;
}
cout << endl << endl;
delete[] packages;
return 0;
}
-
I can run it fine in both Windows and Linux. Are you getting a segfault or an assertion failure? Can you run a debugger on it?
-
I no longer get a segmentation fault, but its not sorting them correctly. Once a priority 3 was printed before a priority 1, then a 2 was first, then 1, then 3; then with everything was sorted correctly....
-
Compare what you've got with what you've posted, and make sure you recompiled -- the above is giving correct results.
-
yeah, i did, and its still not sorting right.
For example, the two text files:
First:
Second:
Code:
1This program is awesome
The second text file entered should come out first, but it doesn't.
-
If you only have two elements in your list, your bubble sort doesn't run (the i loop is immediately false).
-
How so? the first message has a priority of 3 and the second 1, so wouldn't is swap them?
Oh, got it.... so i could just create an if statement to correct this problem? if amount == 2, then another if statement to swap them if the first has less priority than the other?
-
Your bubble sort doesn't run. i begins at 1, amount is 2, and therefore the for loop test fails immediately. (You should have <=, not <, which we managed to miss for all this time.)
-
cool, never mind my edit then.... let me try that.