Value of a variable is not updating, it is either 1 or 0

Question

Hey there! In the following code, I am trying to count frequency of each non zero number

My intention of the code is to update freq after testing each case using nested loop but value of freq is not updating. freq value remains to be either 0 or 1. I tried to debug but still ending up with the same bug.

Code:

#include <bits/stdc++.h>
using namespace std;

int main() {
    int size;
    cin>>size;
    int freq=0;
    int d[size];
    for(int i=0;i<size;i++){ //To create array and store values in it
            cin>>d[i];
    }
    for(int i=0;i<size;i++){
            if(d[i]==0 )continue;
            for(int j=0;j<size;j++){
            if(d[i]==d[j]){
                    freq=freq+1;
                    d[j]=0;
                }
            }
            cout<<"Frequency of number "<<d[i]<<" is "<<freq<<endl;
            d[i]=0;
            freq=0;
    }
}

Input:

5

1 1 2 2 5

Expected output:

Frequency of number 1 is 2


Frequency of number 2 is 2 


Frequency of number 5 is 1

Actual output:

Frequency of number 0 is 1

Frequency of number 0 is 1

Frequency of number 0 is 1

Frequency of number 0 is 1

Frequency of number 0 is 1

Some one please debug the code and fix it. Open for suggestions.

Answer 1

To build an histogram, you actually need to collect history.

Example:

int main() {
    int size;
    cin >> size;

    int d[size];
    int hist[size + 1]{};             // all zeroes - this is for the histogram
    for (int i = 0; i < size; i++) {  // To create array and store values in it
        cin >> d[i];
    }
    for (int i = 0; i < size; i++) {
        ++hist[d[i]];
    }
    for(int i = 0; i < size; ++i) {
        cout << "Frequency of number " << i << " is " << hist[i] << endl;
    }
}

Note: VLAs (Variable Length Arrays) are not a standard C++ feature. Use std::vector instead.

A slightly different approach would be to not be limited by the size parameter when taking the input values. std::map the value to a count instead:

#include <iostream>
#include <vector>
#include <map>

int main() {
    int size;
    if(not (std::cin >> size) or size < 1) return 1;

    std::map<int, unsigned long long> hist; // number to count map

    for(int value; size-- > 0 && std::cin >> value;) {
        ++hist[value];
    }

    for(auto[value, count] : hist) {
        std::cout << "Frequency of number " << value << " is " << count << '\n';
    }
}

Answer 2

#include <bits/stdc++.h>

This is not standard C++. Don't use this. Include individual standard headers as you need them.

using namespace std;

This is a bad habit. Don't use this. Either use individual using declarations for identifiers you need, such as using std::cout;, or just prefix everything standard in your code with std:: (this is what most people prefer).

int d[size];

This is not standard C++. Don't use this. Use std::vector instead.

      for(int j=0;j<size;j++){
      if(d[i]==d[j]){

Assume i == 0. The condition if(d[i]==d[j]) is true when i == j, that is, when j == 0. So the next thing that happens is you zero out d[0].

Now assume i == 1. The condition if(d[i]==d[j]) is true when i == j, that is, when j == 1. So the next thing that happens is you zero out d[1].

Now assume i == 2. The condition if(d[i]==d[j]) is true when i == j, that is, when j == 2. So the next thing that happens is you zero out d[2].

Now assume i == 3 ...

So you zero out every element of the array the first time you see it, and if(d[i]==d[j]) never becomes true when i != j.

This can be fixed by changing the inner loop to

   for (int j = i + 1; j < size; j++) {

This will output freq which is off by one, because this loop doesn't count the first element. Change freq = 0 to freq = 1 to fix that. I recommend having one place where you have freq = 1. A good place to place this assignment is just before the inner loop.

Note, I'm using spaces around operators and you should too. Cramped code is hard to read.

Here is a live demo of your program with all the aforementioned problems fixed. No other changes are made.