Why unordered_map is not giving correct answer every time while finding is there any duplicate value present in array or not?

Question

I have given an array and I need to find is there any duplicate element present in array or not? (I have to take input from the user.)

This is my first code which is giving the wrong answer for some test cases:

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

int main() {
    // your code goes here
    int t;
    cin>>t;
    while(t--)
    {
        int n;
        cin>>n;
        unordered_map<int,int> mp;
        int temp=1;
        for(int i=0,i<n;i++) 
        {
            int x; cin>>x;
            if(mp.find(x)!=mp.end()) 
            {
                temp=-1; break;
            }     
            
            mp[x]++;
        }
        if(temp==-1) cout<<"YES"<<endl; //if duplicate present
        else cout<<"NO"<<endl;
        mp.clear();
        
            
    }
    return 0;
}

And this code is running perfectly on all the test cases:

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

int main() {
    // your code goes here
    int t;
    cin>>t;
    while(t--)
    {
        int n;
        cin>>n;
        unordered_map<int,int> mp;
        int temp=1;
        for(int i=0,i<n;i++)  
        {
            int x; 
            cin>>x;
               
            
            mp[x]++;
        }
        
        if(mp.size()<n) cout<<"YES"<<endl;
        else cout<<"NO"<<endl;
        mp.clear();
        
            
    }
    return 0;
}

what is the reason behind it?

I stopped reading at the `for` macro. If you want someone to read your code then make it readable. — Retired Ninja, Nov 14 '20 at 05:35
@AbhishekKumar I disagree. It makes the code less readable for no good reason. The macro isn't even used in more than one place so it's actually more code to write than the straightforward version that you can read inline without having to go look at what the macro is. As for more knowledge, well I've been doing this for 35 years. I've grown out of my "let's make this clever" phase. — Retired Ninja, Nov 14 '20 at 08:47
@RetiredNinja okay, I have updated it. Thanks for pointing out. — Abhishek Kumar, Nov 14 '20 at 08:52
@AbhishekKumar *Whole code is very simple even for a beginner* -- *So I think you need more knowledge before saying it's not readable* -- You're saying this, but you couldn't find the obvious differences in the code. — PaulMcKenzie, Nov 14 '20 at 09:10
see [Why is “using namespace std;” considered bad practice?](https://stackoverflow.com/q/1452721/995714), [Why should I not #include ?](https://stackoverflow.com/q/31816095/995714) — phuclv, Nov 14 '20 at 13:22

score 2 · Accepted Answer · answered Nov 14 '20 at 05:48

You used break; in the first code, but didn't use that in the second code.

Using break; will prevent the code from reading part of test case and treat that as next text case, and it will lead to Wrong Answer.

For example, using this input

2
5 5 5 1 2 3
2 2 2

Your first code will print

YES
NO

while the correct output is

YES
YES

To prevent this, the break; should be removed.

Why unordered_map is not giving correct answer every time while finding is there any duplicate value present in array or not?

1 Answers1