C++

Forum

 
Bubble Sort

Donut (8)
I am attempting to sort some salary data that I import using bubble sort. However, I am not getting the sorted data out to the screen. What am I missing?


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
 
#include <fstream>
#include <iostream>
#include <cstdlib>
#include <iomanip>

int importFile();
//Prompts the user for the name of the file to import and imports the file.
void calculateMinMaxAvg();
//Calculates the min, max, and avg salary from the imported file and displays those values to the user.
void sortArray(double records[], int numRecords)
{
   using namespace std;
   double temp = 0; 
           
            
		      for(int counter = numRecords; counter > 0; counter --)
				{
					for (int index = 1; index < numRecords; index++)
					{
						if (records[index] > records[index + 1])
						{
							temp = records[index + 1];
							records[index +1] = records[index];
							records[index] = temp;
						} //end inner for loop  
					} //end out for loop
			numRecords--;
				}	           
            for (int index = 1; index < numRecords; index++)
				{ 
					cout << records[index] << ", ";
				}
			cout << endl; 
           
}

const int MAX_MONTHS = 20; //max number of months to import per supervisor

						   

bool isValid;
char rerun;
double total;
double records[MAX_MONTHS]; //supervisor specified the array would have a max value of 20. 
double maxValue;
double minValue;
int numRecords;    //variable to store the total number of records located on the first line of the import file.


int main()
{
	using namespace std;

	do
	{
		
		importFile();
		calculateMinMaxAvg();
		sortArray(records, numRecords);


		cout << "Would you like to run this again? (y/n)\n";
		cin >> rerun;
		if (rerun == 'y' || rerun == 'Y' || rerun == 'n' || rerun == 'N')
			isValid = true;
		else
			cout << "Valid input is Y for YES or N for NO \n";
	} while (isValid = true);
	{
		cout << "Goodbye! \n";
	}
	return 0;
}

int importFile()
{
	using namespace std;

	char infilename[16];
	


	ifstream inStream;
	ofstream outStream;
	cout << "Enter the input file name (maximum of 15 characters):\n";
	cin >> infilename;
	cout << "This application will read the salary data from the file and compute low, high, and average salary amounts. \n";
	total = 0; //initialize accumulator to 0 inside of loop
	inStream.open(infilename);
	if (inStream.good())
	{
		inStream >> numRecords;
		cout << "This file contains " << numRecords << " months worth of salary data." << endl;
		if (numRecords > MAX_MONTHS)
		{
			cout << "Maximum number of records to be imported is " << MAX_MONTHS << ". Please adjust your input file named " << infilename << " and rerun the import.\n";
			return 0;
		}
		else;
		inStream >> records[0];
		minValue = records[0];
		maxValue = records[0];
		for (int i = 1; i < MAX_MONTHS && i < numRecords; i++)
		{
			inStream >> records[i];
		}
	}
	inStream.close();
}
void calculateMinMaxAvg()
{
	using namespace std;
   

	
	for (int i = 0; i < MAX_MONTHS && i < numRecords; i++)
	{
		total = total + records[i];

		if (minValue > records[i])
		{
			minValue = records[i];
		}
		else if (maxValue < records[i])
		{
			maxValue = records[i];
		}
	}



	cout.setf(ios::fixed);
	cout.setf(ios::showpoint);
	cout.precision(2);
	cout << "The lowest monthly salary earned was $" << minValue << endl;
	cout << "The highest monthly salary earned was $" << maxValue << endl;
	cout << "The average monthly salary earned was $" << total / numRecords << endl << endl;
	cout << "The monthly salary data has been sorted in ascending order for your review: "<<endl;
  
   
   }
Enoizat (1343)
Hi, Donut.
I’ve never seen that version of Bubble Sort before and I could be wrong, but, for what I can see, you could go out of boundaries both here:

for(int counter = numRecords; counter > 0; counter --)

and here:

1
2
 
... index < numRecords; ...
   if (records[index] > records[index + 1])


I think the simplest solution is to output what your sorting algorithm does while it executes and in case check online for a more traditional version of Bubble Sort.

There’re other problems in you code. You can check this version for hints:
file Donut.txt

20
89
53
84
82
35
61
33
8
99
69
51
58
36
87
19
16
16
4
68
26

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
 
#include <cstdlib>  // c'mon! Use std::string instead
#include <fstream>
#include <iomanip>
#include <iostream>
#include <limits>

constexpr std::size_t MAX_FILENAME { 16 };
constexpr int MAX_MONTHS = 20; // max number of months to import per supervisor

char* askForFileName(char* const fname, const char* const info, std::size_t size);

int importRecords(const char* const fname, int max_recs, int* const records);

int calculateMinMaxAvg(const int* const records, int size,
                       int& min, int& max, double& avg);

void displayMinMaxAvg(int min, int max, double avg,
                      const int* const records, int size);

void sortArray(int* const records, int size);

int main()
{
    char rerun {};
    do {
        std::cout << "This application will read the salary data from the file "
                     "and compute low, high, and average salary amounts.\n";

        char fname[MAX_FILENAME];
        askForFileName(fname, " - max characters: ", MAX_FILENAME - 1);

        int records[MAX_MONTHS];    // use std::vector instead!
        int reads = importRecords(fname, MAX_MONTHS, records);

        int min {}, max {};
        double average {};
        calculateMinMaxAvg(records, reads, min, max, average);

        sortArray(records, reads);
        displayMinMaxAvg(min, max, average, records, reads);

        std::cout << "Would you like to run this again (y/n) ?";
        std::cin >> rerun;
    } while (rerun == 'y' || rerun == 'Y');

    std::cout << "Goodbye!\n";
    return 0;
}

char* askForFileName(char* const fname, const char* const info, std::size_t size)
{
    std::cout << "Please, enter the input file name" << info << size << ": ";
    std::cin >> fname;
    std::cin.ignore(1);
    return fname;
}

int importRecords(const char* const fname, int max_recs, int* const records)
{
    std::ifstream in_stream(fname);
    if(!in_stream) { return 0; }

    // First datum: number of existing records
    int num_records;
    in_stream >> num_records;
    std::cout << "The file " << fname << " contains " << num_records
              << " months worth of salary data.\n";
    if (num_records > max_recs)
    {
        std::cout << "File error.\nMaximum number of records to be imported: "
                  << max_recs
                  << ".\nPlease provide a different file name.\n";
        return 0;
    }

    for(int i {}; i < num_records && in_stream >> records[i]; ++i) {}

    in_stream.close();          // reduntant

    return num_records;         // assumes no errors inside file!!!
}


int calculateMinMaxAvg(const int* const records, int size,
                       int& min, int& max, double& avg)
{
    double total {};
    min = std::numeric_limits<int>::max();
    max = std::numeric_limits<int>::min();

    for (int i = 0; i < size; ++i)
    {
        total += records[i];
        if (min > records[i]) { min = records[i]; }
        if (max < records[i]) { max = records[i]; }
    }
    
    avg = total / static_cast<double>(size);

    return 0;   // for further improvements, e.g. returning error values
}               // This is personal: you can safely make this function void


void displayMinMaxAvg(int min, int max, double avg,
                      const int* const records, int size)
{
    std::cout.setf(std::ios::fixed);
    std::cout.setf(std::ios::showpoint);
    std::cout.precision(2);
    std::cout << "The lowest monthly salary earned was $" << min
              << ".\nThe highest monthly salary earned was $" << max
              << ".\nThe average monthly salary earned was $" << avg
              << ".\n\nThe monthly salary data has been sorted in ascending "
                 "order for your review:\n";

    for(int i {}; i < size; ++i) {
        std::cout << records[i] << ", ";
    }
    std::cout << '\n';
}

void sortArray(int* const records, int size)
{
    std::cout << "size: " << size << '\n';
    for(int counter = size; counter > 0; --counter)
    {
        for (int index = 1; index < size; ++index)
        {
            if (records[index] > records[index + 1])
            {
                std::cout << "counter: " << counter << "; "
                          << "swapping records[" << index + 1
                          << "] (" << records[index + 1]
                          << ") and record[" << index
                          << "] (" << records[index] << ")\n";
                int temp = records[index + 1];
                records[index + 1] = records[index];
                records[index] = temp;
            }
        } // end inner for-loop
        size--;
    } // end outer for-loop
}

Last edited on
Donut (8)
Thanks Enoizat! I am self taught in C++ and learning the hard way...I wasnt looking for work when an employee left our group but someone has to do it. LOL. I guess it is time to brush up on some of the concepts in your code versus the code I was handed. Thanks again!
jlb (4973)
I guess it is time to brush up on some of the concepts in your code versus the code I was handed.

I'd also suggest you consider using std::vector instead of the arrays and use std::string instead of the C-strings. Then it would be easier to use std::sort to sort the records instead of making your own sort routine.

Oh and while you're at it, stop using all of those global variables, learn to properly pass variables to and from your functions.

Donut (8)
Thanks again for the pointers...As i said I am self taught on C++ and have only been exposed to it since June. I am a route/switch guy that was assigned the temp programming tasks while we try to fill the empty spot. Hopefully, that day will come soon.

I got the bubble sort to work with some additional reading but I see what Enoizat was saying...if I import a file with more than 20 months worth of salary it tells the user "Maximum number of records to be imported is " <<MAX_MONTHS<<". Please adjust your input file name "<<infilename<< "and rerun the import." but then the program continues on instead of looping back to "enter the input file name" which is what is desired. The other issue I noticed is that when it prompts the user do you want to run again, it doesnt matter what the user enters. This application all worked fine while it was in main (), but I tried to break it into functions so it was more modular and now I broke it, LOL.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
 
#include <fstream>
#include <iostream>
#include <cstdlib>
#include <iomanip>

int importFile();
//Prompts the user for the name of the file to import and imports the file.
void calculateMinMaxAvg();
//Calculates the min, max, and avg salary from the imported file and displays those values to the user.
void sortArray(double records[], int numRecords)
{
   using namespace std;
		int temp = 0; 
		bool swapped = true;
        int j = 0;
		while (swapped){
			swapped = false;
			j++;
			for (int i = 0; i < numRecords - j; i++){
					if (records[i] > records [i+1]) {
						temp = records[i];
						records[i] = records [i+1];
						records [i+1] = temp;
						swapped = true;
					}
				}
			}	           
            for (int index = 1; index < numRecords; index++)
				{ 
					cout << records[index] << ", ";
				}
			cout << endl; 
           
}

const int MAX_MONTHS = 20; //max number of months to import per supervisor

						   

bool isValid;
char rerun;
double total;
double records[MAX_MONTHS]; //supervisor specified the array would have a max value of 20. 
double maxValue;
double minValue;
int numRecords;    //variable to store the total number of records located on the first line of the import file.


int main()
{
	using namespace std;

	do
	{
		
		importFile();
		calculateMinMaxAvg();
		sortArray(records, numRecords);


		cout << "Would you like to run this again? (y/n)\n";
		cin >> rerun;
		if (rerun == 'y' || rerun == 'Y' || rerun == 'n' || rerun == 'N')
			isValid = true;
		else
			cout << "Valid input is Y for YES or N for NO \n";
	} while (isValid = true);
	{
		cout << "Goodbye! \n";
	}
	return 0;
}

int importFile()
{
	using namespace std;

	char infilename[16];
	


	ifstream inStream;
	ofstream outStream;
	cout << "Enter the input file name (maximum of 15 characters):\n";
	cin >> infilename;
	cout << "This application will read the salary data from the file and compute low, high, and average salary amounts. \n";
	total = 0; //initialize accumulator to 0 inside of loop
	inStream.open(infilename);
	if (inStream.good())
	{
		inStream >> numRecords;
		cout << "This file contains " << numRecords << " months worth of salary data." << endl;
		if (numRecords > MAX_MONTHS)
		{
			cout << "Maximum number of records to be imported is " << MAX_MONTHS << ". Please adjust your input file named " << infilename << " and rerun the import.\n";
			return 0;
		}
		else;
		inStream >> records[0];
		minValue = records[0];
		maxValue = records[0];
		for (int i = 1; i < MAX_MONTHS && i < numRecords; i++)
		{
			inStream >> records[i];
		}
	}
	inStream.close();
}
void calculateMinMaxAvg()
{
	using namespace std;
   

	
	for (int i = 0; i < MAX_MONTHS && i < numRecords; i++)
	{
		total = total + records[i];

		if (minValue > records[i])
		{
			minValue = records[i];
		}
		else if (maxValue < records[i])
		{
			maxValue = records[i];
		}
	}



	cout.setf(ios::fixed);
	cout.setf(ios::showpoint);
	cout.precision(2);
	cout << "The lowest monthly salary earned was $" << minValue << endl;
	cout << "The highest monthly salary earned was $" << maxValue << endl;
	cout << "The average monthly salary earned was $" << total / numRecords << endl << endl;
	cout << "The monthly salary data has been sorted in ascending order for your review: "<<endl;
  
   
   }
jlb (4973)
As I said earlier you may want to consider using C++ features like std::vector and std::sort. For example your sort could look more like:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
 
#include <iostream>
#include <algorithm>

void sortArray(double records[], int numRecords);
void printArray(double records[], int numRecords);

int main()
{
    double records[4]{1.2, 6.5, 4.2, 2};

    printArray(records, 4);

    sortArray(records, 4);

    printArray(records, 4);

}

void printArray(double records[], int numRecords)
{
    for(auto& itr : records)
        std::cout << itr << ' ';
    std::cout << '\n'
}

void sortArray(double records[], int numRecords)
{
    std::sort(records, records+numRecords);
}


And notice the lack of any global variables.

Ganado (6813)
jlb, if Donut is trying to learn how sorting actually works, it doesn't help much to just call std::sort. Just like how using a calculator doesn't help you learn how to do division. Of course, for purposes other than learning, he should use std::sort, but if he's learning about different sorting algorithms, then it's best to program and test those particular sorting algorithms. (Global variables should be avoided either way, I agree.)

Donut, be careful of "off-by-1" errors. Arrays start at index 0, not 1. Look at lines 28 and 30.
Also, look at line 67. You should use == to test for equality. = does assignment, which you don't want here.
You actually can just do, while (isValid); because isValid is already a bool.

Look at line 99: else;
If you put a semi-colon after an if or else statement, it cuts it off right then and there, and the line below it will always execute.
As a beginner, you should get in the habit of always putting { and } around your if statements.

1
2
3
4
5
6
7
8
9
10
 
if (something)
{
    do_this();
}
else
{
    do_that();
    and_that(42);
}
always_do_this();
Last edited on
jlb (4973)
jlb, if Donut is trying to learn how sorting actually works, it doesn't help much to just call std::sort.

Sometimes it is more important to learn the proper way of doing things, especially when you're getting paid to do some task. Note this quote from a previous post.

Thanks Enoizat! I am self taught in C++ and learning the hard way...I wasnt looking for work when an employee left our group but someone has to do it. LOL. I guess it is time to brush up on some of the concepts in your code versus the code I was handed. Thanks again!



Ganado (6813)
Oh, I see. Then it's good you showed him the correct way. Although, my philosophy would be "why not both" :)
Last edited on
Donut (8)
Thanks again for your help and guidance. As I stated previously I am mostly a route switch guy but in a small office sometimes we wear several hats. I have no formal programming training or education which is obvious by my coding snippets. LOL

I have learned a lot in the 6 weeks I have had to learn C++ and much of the credit for me being able to do anything in C++ goes to contributors like yourself and countless hours reading what you guys offer.

Thanks again...hopefully we fill the spot soon and I can go back to only wearing 6 hats instead of 8.
Topic archived. No new replies allowed.