How do I return a value of -1 if there are no duplicate values

This code is supposed to find the value with the most occurences. It's supposed to return a -1 if the their were no duplicate values. Here is what I came up with, but I can never get it to return a -1. Is this working or did I do something wrong?

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
  #include <iostream>
#include <ctime>
#include <cstdlib>
using namespace std;

int GetMostValue(int* arr, const int SIZE)
{
	int value = -1,
	max_count = 0;
	int* ptr1;
	int* ptr2;
	ptr1 = arr;
	ptr2 = ptr1 + 1;


	for (int i = 0; i < SIZE; i++)
	{
		int count = 1;
		for (int j = 1; j < SIZE; j++)
		
			if(&ptr1 == &ptr2)
				count++;
		
		if(count > max_count)
			max_count = count;	

		ptr1 = ptr1 + 1;
		ptr2 = ptr2 + 1;
		
	}

	ptr1 = arr;
	ptr2 = ptr1 + 1;

	for (int i = 0; i < SIZE; i++)
	{
		int count = 1;
		for (int j = 1; j < SIZE; j++)
			if (ptr1 == ptr2)
				count++;
		if (count == max_count)
			value = arr[i];
		ptr1 = ptr1 + 1;
		ptr2 = ptr2 + 1;
	}

	return value;
}

int main()
{
	srand(time(0));
	const int SIZE = 100;
	int arr[SIZE];
	int value = 0;
	for (int i = 0; i < SIZE; i++)
	{
		
		arr[i] = rand() % 100 + 1;

	}

	value = GetMostValue(arr, SIZE);
	cout << value;
	return(0);
	
}
line 21 looks wrong. you are comparing addresses. I think you want * instead of &.
and again on 39, you compare addresses.
either I don't get it or you need to review how to use a pointer (or, maybe better, stop using pointers). You can do all this with array index / integers instead of pointers with less weirdness.

the easy way out is to sort the data, then just iterate it one time, all your duplicates will be together thanks to the sorting.

if you don't want to do that, you need to do this:
for everything in the array, check it against *everything else* in the array, but not itself. if you accidentally check it against itself you register a false duplicate and won't ever get -1 for sure.

here is a condensed version without pointers (apart from passing the array, which is a pointer no matter what color you use)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22

int ctdup(int *ip, int sizeip)
{
	int ret{-1}; //default value. if no dupes found, this will be returned. 
	int ct; //count most common item
	int oldct{};//previous count
	for(int i = 0; i < sizeip; i++) //for each one
	{
		ct = 0; //reset the counter
		for(int j = 0; j < sizeip; j++) //check each one against all the others
		{
		  if(j!=i && ip[j]==ip[i]) //if its not the same item, and they are identical
                     ct++; //count it
		}		
		if(ct && ct > oldct) //if have any duplicates for current item outer loop
		{                    //and we have more than previous highest count
		  ret = ip[i];      //save the current item as most popular
		  oldct = ct;       //save its count 
		}
	}
	return ret; 
}

the whole thing would be smaller if you used vectors, if you know those yet. Vectors are smarter arrays. It would be smaller and faster both if you sorted first, which gets rid of a lot of the junk.
Last edited on
SIZE is only 100. Probably easiest just to use a 100-element frequency array.
Using vector/map etc rather than c-style array and C++ random rather than c rand(), then possibly:

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
#include <iostream>
#include <algorithm>
#include <map>
#include <vector>
#include <random>

std::mt19937 rng(std::random_device {}());
constexpr size_t NoNums {100};

int getMostValue(const std::vector<int>& arr) {
	std::map<int, unsigned> cnts;

	for (const auto& a : arr)
		++cnts[a];

	const auto me {std::max_element(cnts.begin(), cnts.end(), [](const auto& a, const auto& b) { return a.second < b.second; })};

	return me->second > 1 ? me->first : -1;
}

int main() {
	std::vector<int> arr(NoNums);
	std::uniform_int_distribution<int> distrib(1, 100);

	for (auto& elem : arr)
		elem = distrib(rng);

	std::cout << getMostValue(arr);
}

If you take 100 values ... each between 1 and 100 ... the chance of no repeats is pretty small (about 9x10-43 in fact). So, it might be a while before you return a -1.

You can try turning SIZE down in the following, but note that it only returns one "most-frequent" and in many cases your array values will be multi-modal. What do you want to do in that case?

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 <ctime>
#include <cstdlib>
using namespace std;

int GetMostValue( int *arr, int arrSize, int limit )
{
   int *freq = new int[1+limit]{};
   for ( int i = 0; i < arrSize; i++ ) freq[arr[i]]++;

   int posMaxFreq = 0;
   for ( int j = 1; j <= limit; j++ ) if ( freq[j] > freq[posMaxFreq] ) posMaxFreq = j;
   if ( freq[posMaxFreq] == 1 ) posMaxFreq = -1;

   delete [] freq;
   return posMaxFreq;
}

int main()
{
   srand(time(0));
   const int SIZE = 10, LIMIT = 10;
   int arr[SIZE];
   for ( int i = 0; i < SIZE; i++ )
   {
       arr[i] = rand() % LIMIT + 1;
       cout << arr[i] << ' ';
   }
   cout << '\n' << "Most popular: " << GetMostValue( arr, SIZE, SIZE );
}


9 4 1 10 4 9 9 8 5 7 
Most popular: 9 


If you turn SIZE and LIMIT both down to 3 then you should get no repeats about 2 times in 9.
Last edited on
Topic archived. No new replies allowed.