Review my Source Code On Finding the Mode

I was doing a side project from my textbook about finding the mode, which is the number that occurs most often in a sequence of numbers. I racked my brain on this for an embarrassing amount of time and finally came up with this, which I think works but for some reason I still feel like I'm not done. Tell me what you think and what you would have done differently!

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
#include "stdafx.h"
#include <iostream>

using namespace std;
void mode(int *, int);

void main()
{
	const int NUMBERS = 15;
	int list[NUMBERS] = {99,73,56,14,28,42,93,64,51,26,56,16,38,81,98};
	mode(list,	NUMBERS);
}

void mode(int *arr, int SIZE)
{
	int countmode = 0;
	int finalcount = 0;
	int mode = 0;

	for(int count = 0; count < SIZE; count++)
	{
		countmode = 0;

		for(int i = 0; i < (SIZE); i++)
		{
			if(arr[count] == arr[i])
			{
			    countmode++;
			}
		}

		countmode = countmode - 1;
//because the arr[count] will be compared against it's own value once every
 //iteration of the outer for loop which doesn't count toward the modecount

		if(countmode > finalcount)
		{
			finalcount = countmode;
			mode = arr[count];
		}	
		
	}
	if(mode > 0)
		cout << "Final Mode " << mode << endl;
	else
		cout << "There is no mode." << endl;
}


Console Output:
1
2
Final Mode 56
Press any key to continue . . .


There it is, I would have commented more but I couldnt think of the right words to explain everything. If you have questions ask please, I just want to make sure I'm doing this right!
1) void main() is illegal C++. Main should return int. Some older compilers accept it, but you should not use it.

2) Avoid using namespace std. It can bring problems in the future.

3) Do not overuse endl. Use only if you really need immediate buffer flush (which is almost never). If used in a loop it can cause significant slowdown for IO operations.

4) Your code has a bug. Replace list with this and see for yourself: int list[NUMBERS] = {99,73,56,14,28,0,0,0,0,0,0,0,0,0,0};

5) Use C++ managed containers instead of C arrays. Vector (or std::array) would be good here.

6) Use standard library instead of reinventing your own algorithms:

Program with some parts directly replaced with standard library ones. No change to program logic was made:
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
#include <iostream>
#include <algorithm>
#include <vector>

void mode(const std::vector<int>& arr)
{
	int finalcount = 0;
	int mode = 0;
	for(int count = 0; count < arr.size(); ++count)	{
		int countmode = std::count(arr.begin(), arr.end(), arr[count]) - 1;
		if(countmode > finalcount) {
			finalcount = countmode;
			mode = arr[count];
		}
	}
	if(mode > 0)
		std::cout << "Final Mode " << mode << '\n';
	else
		std::cout << "There is no mode.\n";
}

int main()
{
	std::vector<int> list = {99,73,56,14,28,42,93,64,51,26,56,16,38,81,98};
	mode(list);
}

Last edited on
what MiiniPaa said plus:
i also think it's not great to use the same name for methods and variables (in your case 'mode'), and get rid of this:
#include "stdafx.h"

Use standard library instead of reinventing your own algorithms

I disagree with this if someone is learning c++. It's good practice to sort out the logic. Although a knowledge of them is good.
I disagree with this if someone is learning c++
Write someone once, understand how it works, use standard library alternative later. I doubt that writing another loop to count entries will teach somebody anything. Is something it will make people to became lost in many loops, unnessesary complexivity and low-level manipulations, when replacing redundant parts with standard alternatives will increase readability and train standard library knowledge (very neglected part of C++ learning)

I didn't even pointed that this function can be written in seven lines, as function itself is probably is a practice. Counting elements is really below that level, so it has no teaching purpose now.
Wow, thanks a lot guys! I don't think I've learned many of the concepts your talking about yet because I studied very carefully and haven't come across them yet, I used the C style array because that's all I've learned thus far, I don't know what you mean by"use the std library instead of reinventing your own algorithms", but I'm sure I will learn, and I've dabbled with the vector but I guess now is a good time to start using it so thanks a lot this was helpful!
Topic archived. No new replies allowed.