Recently I see more and more answers here on SO that lead users in the wrong direction by giving bad answers.
Also, for C++, the question has already been answered in the comment by Igor Tandetnik, and that should finally be used.
But let me answer the question of the OP as asked. What is wrong with my function? OK, there are several aspects. Let us first look at the style.
You have 0 lines of comments, so the code quality is 0. If you would write comments, then you would already find most bugs by yourself, because then, you need to explain your own wrong statements.
Then please see your source code with my amendments. I added the problems as comment.
// This is just a dumped function and not a minimum reproducible example
// All header files are messing
// Obviously "using namespace std;" was used that should NEVER be done
// The function should retrun an unsigned value, best size_t, because a count can never be negative
// Same for n, that is the size of an array. Can also never be negative
// C-sytle arrays should NEVER be used in C++. NEVER. Use std::vector or std::array instead
int countDifferentNumbers(int v[], int n)
{
int counter = 0; // Now in C++ we can use braced initialzation instead of assignement
for (int i = 0; i < n; ++i)
{
for (int j = i; j < n; ++j)
{
if (v[i] == v[j + 1]) // Accessing out of bounds element
{
cout << "match" << endl; // Now endl needed here. Can all be done in one cout statement in one line
counter++; // Always counting up the same counter for all kind of double numbers.
cout << v[i] << endl;
}
}
}
return counter;
That was one point of the answer. But now the second point. Evene more important. The algorithm or the design is wrong. And finding the correct solution, this thinking before codingt, you need to do, before you write any line of code.
You obviously want to find the count of unique numbers in an array.
Then you could look what is already there on Stackoverflow. You would probaly find 20 answers already that coud give you a hint.
You could use std::unique. Please see here for a description. This function sounds like it does what you want, right? Some example implementation:
#include <iostream>
#include <unordered_map>
#include <vector>
#include <algorithm>
// If you want to keep the original data, remove the reference-specifier &
size_t countDifferentNumbers(std::vector<int>& v) {
std::sort(v.begin(), v.end()); // Sorting is precondition for std::unique
v.erase(std::unique(v.begin(), v.end()), v.end()); // Erase all non-unique elements
return v.size(); // Return the result
}
int main() {
std::vector test{ 1, 5, 5, 8, 1, 1 }; // Some test data
std::cout << countDifferentNumbers(test) << '
'; // SHow result to user
return 0;
}
Then, we could count the occurence of each number in a std::map or std::unordered_map. And the number of counters will be the result. Example:
#include <iostream>
#include <unordered_map>
#include <vector>
#include <algorithm>
// If you want to keep the original data, remove the reference-specifier &
size_t countDifferentNumbers(std::vector<int>& v) {
std::unordered_map<int, size_t> counter{}; // Here we will count all occurences of different numbers
for (const int i : v) counter[i]++; // Iterate over vector and count different numbers
return counter.size(); // Count of different numbers
}
int main() {
std::vector test{ 1, 5, 5, 8, 1, 1 }; // Some test data
std::cout << countDifferentNumbers(test) << '
'; // Show result to user
return 0;
}
But, then, thinking further, about what conatiners we could use, we will find out the answer from Igor Tandetnik. There are 2 containers that can hold unique values only. No double values. And these are: std::set and std::unordered_set., So, we can simply copy the data into one of those containers, and, only unique values will be stored there.
There are many ways to get the data into a set. But the simplest one is to use its range constructor. Then, we have unique elements, and, the containers size function will give the result:
See here: Constructor Number 2.
The result will be a function with one line like this
#include <iostream>
#include <unordered_set>
#include <vector>
// If you want to keep the original data, remove the reference-specifier &
size_t countDifferentNumbers(std::vector<int>& v) {
return std::unordered_set<int>(v.begin(), v.end()).size();
}
int main() {
std::vector test{ 1, 5, 5, 8, 1, 1 }; // Some test data
std::cout << countDifferentNumbers(test) << '
'; // Show result to user
return 0;
}
And since functions with one line are often not so usefull, we can also write the final solution:
#include <iostream>
#include <unordered_set>
#include <vector>
int main() {
std::vector test{ 1, 5, 5, 8, 1, 1 }; // Some test data
std::cout << std::unordered_set<int>(test.begin(), test.end()).size() << '
'; // Show result to user
return 0;
}
So, by analyzing the problem and choosing the right algorithm and container and using C++, we come to the most easy solution.
Please enable C++17 for your compiler.
