Sorting Problem

[kgNIU]

Basically this is suppose to read zipcodes from a file and sorts them in ascending order then calculates the total amount processed. I am getting output, but I think my problem is somewhere in my sort function. The program pretty much runs down to the last zipcode in the file and prints it out and the rest come out as just 0's. What am I doing wrong?

#include <algorithm>
#include <iostream>
#include <fstream>
#include <cstdlib>
#include <cstring>
#include <iomanip>
using namespace std;



const int L = 100;


struct City
{
int zip_code;
int count;
double percent;
};



City Ar[L];
int buildArray( City [] );
int calcPerc( City [], int);
void printArray( City [], int, int);
void sortArray( City [], int );



int main()
{
        int num;
        int totalsales;


        num = buildArray(Ar);
        totalsales = calcPerc(Ar, num);
        printArray(Ar, num, totalsales);
        sortArray(Ar, num);

return 0;
}

int buildArray( City Ar[] )

{

ifstream inFile;

int temp = 0, number = 0, i;

inFile.open("zipcodes.txt");

inFile >> temp;

while(inFile)
{

                 for(i = 0; i < L; i++)
                 {

                       if(temp == Ar[i].zip_code){

                           Ar[i].count++;
                           break;}


                       Ar[i].zip_code = temp;

                       Ar[i].count = 1;

                       break;

                 }

                 inFile >> temp;

                 number++;
    }

    inFile.close();

    return number;

}


int calcPerc( City Ar[], int num)
{
        int i;
        int totalsales;

        for(i=0; i < L; i++)
                totalsales = totalsales + Ar[i].count;

        for(i=0; i < L; i++)
                Ar[i].percent = (Ar[i].count / totalsales) * 100;

        return totalsales;

}


void printArray( City Ar[], int num, int totalsales )
{
        int i;

        cout << fixed << setprecision(2) << showpoint;
        cout << setw(15) << "Zip Code" << setw(15) << "Count" << setw(15) << "Percent" << endl;

        for(i = 0; i < L; i++)
        cout << setw(15) << Ar[i].zip_code << setw(15) << Ar[i].count
        << setw(15) << Ar[i].percent << endl;

        cout << "Total Sales Processed: " << totalsales << endl;

}

void sortArray( City Ar[], int num )
{
        int i;
        int min;
        int j;
        for(i = 0; i < L; i++)
        {
                min = i;
        for(j = i+1; j < L; j++)
                {
                        if(Ar[j].zip_code < Ar[min].zip_code)
                                min = j;
                }
        swap(Ar[i], Ar[min]);
        }
}

[kgNIU]

#include <algorithm>
#include <iostream>
#include <fstream>
#include <cstdlib>
#include <cstring>
#include <iomanip>
using namespace std;



const int L = 100;


struct City
{
int zip_code;
int count;
double percent;
};



City Ar[L];
int buildArray( City [] );
int calcPerc( City [], int);
void printArray( City [], int, int);
void sortArray( City [], int );



int main()
{
int num;
int totalsales;


num = buildArray(Ar);
totalsales = calcPerc(Ar, num);
printArray(Ar, num, totalsales);
sortArray(Ar, num);

return 0;
}

int buildArray( City Ar[] )

{

ifstream inFile;

int temp = 0, number = 0, i;

inFile.open("zipcodes.txt");

inFile >> temp;

while(inFile)
{

for(i = 0; i < L; i++)
{

if(temp == Ar[i].zip_code){

Ar[i].count++;
break;}


Ar[i].zip_code = temp;

Ar[i].count = 1;

break;

}

inFile >> temp;

number++;
}

inFile.close();

return number;

}


int calcPerc( City Ar[], int num)
{
int i;
int totalsales;

for(i=0; i < L; i++)
totalsales = totalsales + Ar[i].count;

for(i=0; i < L; i++)
Ar[i].percent = (Ar[i].count / totalsales) * 100;

return totalsales;

}


void printArray( City Ar[], int num, int totalsales )
{
int i;

cout << fixed << setprecision(2) << showpoint;
cout << setw(15) << "Zip Code" << setw(15) << "Count" << setw(15) << "Percent" << endl;

for(i = 0; i < L; i++)
cout << setw(15) << Ar[i].zip_code << setw(15) << Ar[i].count
<< setw(15) << Ar[i].percent << endl;

cout << "Total Sales Processed: " << totalsales << endl;

}

void sortArray( City Ar[], int num )
{
int i;
int min;
int j;
for(i = 0; i < L; i++)
{
min = i;
for(j = i+1; j < L; j++)
{
if(Ar[j].zip_code < Ar[min].zip_code)
min = j;
}
swap(Ar[i], Ar[min]);
}
}

[VernonDozier]

Some possibilities:

1. Data is read in incorrectly.
2. Data is sorted incorrectly.
3. Data is displayed incorrectly.

Stick a call to your display before and after the sort. If it's all 0's BEFORE the sort, then ignore the sort function. If it looks good BEFORE the sort, but after the sort, you get 0's, look at the sort.

If it doesn't look good before the sort, try hard-coding the values (i.e. not from the file). If that looks good, then you're reading it in wrong.

So pin down the broad category of where things go wrong first, then get more specific from there.

[kgNIU]

I tried that about 20 minutes ago and I think you're right that it's probably my build function. I've been messing with that for well over 40 minutes now and I still get similar results.

[VernonDozier]

One, format the code. It's impossible to read without indentation. Visual Studio and NetBeans are free and both have automated formatters.

Two, I don't know what the file looks like or what the data is supposed to look like, but I can guess and the buildArray function is almost certainly wrong. You should provide a few records from the file and the corresponding data structures after reading it in. Anyway, you are using uninitialized variables in this program in more than one place. If they are supposed to equal 0, initialize them to 0.

I guess the file is supposed to look like this:

95405
95402
95404
95402
95405

and you'd end up with 3 records:

Record 1
zip_code 95405
count 2

Record 2
zip_code 95402
count 2

Record 3
zip_code 95404
count 1

Something like that, except the file has 100 zips, not 5? If so, you need to read in a number and see if it's already there. If so, increment count for that record. If not, increment number. It sort of looks like that's what you're trying to do. But everything's uninitialized and unindented and you haven't posted the file, so who knows? First thing, check the return value and make sure it's right.

[kgNIU]

It doesn't really matter what's in the file, it's just 560 random zip codes that's all.
I'm pretty sure I've initialized all the variables that I need to unless I missed something.

[VernonDozier]

>> It doesn't really matter what's in the file, it's just 560 random zip codes that's all.

It matters. We have no idea what the file looks like. We don't know if there are repeats, how you're supposed to handle them, etc. We can make an educated guess, but it's a guess till you tell us.

>>I'm pretty sure I've initialized all the variables that I need to unless I missed something.

You have not initialized all the variables. totalsales for one.

But that's not your main problem. Indent your for-loop so you can see what's inside of it and it'll stand out better. You have a loop that isn't a loop due to improper use of the break statement, among other problems.

[kgNIU]

There are duplicate zip codes in the file, another problem I need to fix is after the file is processed no duplicates should show up. Here is the actual file that I'm working with if that helps, http://faculty.cs.niu.edu/~byrnes/csci241/programs/zipcodes.txt

[VernonDozier]

There are duplicate zip codes in the file, another problem I need to fix is after the file is processed no duplicates should show up.

Read them in correctly and there will be no duplicates after they are read in. This all happens before the sort. You're on the right track, but fix those bad break statements.

Algorithm is this:

1. Initialize the number of distinct records to 0.
2. Set up a loop.
3. Condition to get out of the loop is when the file has all been read in.
4. Read a zip code into an integer variable.
5. Search through the array of distinct zip codes. See if the zip is already in there.
6. If it IS NOT already in the array, add that zip code to the array, set its count to 1, then increment the number of distinct zip codes.
7. If it IS already in the array, increment the count for that particular zip code.

The search code needs to be a nested loop within your other loop. It looks like you are attempting something like the algorithm above, but again, due to the way you use the break statement, this isn't the way the code reads.

[kgNIU]

Ok I changed up the code a little bit and made some progress at least. It's now sorting through 5 of the zipcodes then returns the rest as all 0's..

I added an else and took out the break after Ar.count++. It seems to be going in the right direction but I'm still stuck, if I take out that last break statement the program just outputs 70025 for everything with a count of 1.

New Build Code:

int buildArray( City Ar[] )

{

ifstream inFile;

int temp = 0;
int number = 0;
int i;

inFile.open("zipcodes.txt");

inFile >> temp;

while(inFile)
{

        for(i = 0; i < L; i++)
        {

                if(temp == Ar[i].zip_code)
                {

                        Ar[i].count++;


                }



                else
                {
                Ar[i].zip_code = temp;
                Ar[i].count = 1;
                
                break;
                }

}
inFile >> temp;
number++;
}


    inFile.close();

    return number;

}

Output:

Zip Code Count Percent
0 0 0.00
0 0 0.00
19001 2 0.00
19001 1 0.00
20074 1 0.00
25078 1 0.00
70025 1 0.00
there are a ton more 0's at the top of the output but I just cut them out to save space.

[VernonDozier]

there are a ton more 0's at the top of the output

That's the problem. "tons" is overwhelming. Get a file that has 5 zip codes, not 500, and get that right. 500 is too overwhelming. In fact, don't even star with 5. Start with 0 then 1, then 2, then 3...

What does a zip code of 0 represent? Is 0 a legitimate zip code? Initialize the array to something that's NOT legitimate and you'll be able to spot the problem faster. If they are 0 on purpose, fine. But if not, make sure you know what 0 represents.

What do L and number represent? Rename them to something more specific and the code wil be easier to debug and to follow.

The code is broken BEFORE you get to any break statements. Say I have a file. First 2 zips are 90000 and 90001.

Let's say the first reads in correctly, so Ar[0].zip_code = 90000 and Ar[0].count = 1.

Now 90001 is read into temp. I'm on my first trip through the loop, so i = 0, 90001 does not equal 90000, so I end up in the else statement.

Recall that i is 0 and look at this line:

Ar[i].zip_code = temp;

You are overwriting the 90000 with 90001, so it's lost, break or no break. The overall logic needs to be fixed. Rename the variables and it'll be much easier.