-3

Question: Smallest Positive missing number

What is wrong with this code?

class Solution
{
    public:
    //Function to find the smallest positive number missing from the array.
    int missingNumber(int arr[], int n) 
    { 
        // Your code here
        sort(arr,arr+n);
        int index=1;
        int a;
        for(int i=0;i<n;i++){
            if(arr[i]>0){
                if(arr[i]!=index){
                    a=index;
                    break;
                }
                else{
                    index++;
                }
            }
        }
        return index;
    } 
};
John Kugelman
  • 349,597
  • 67
  • 533
  • 578
SAFA
  • 13
  • 2
    Have you done any debugging? What inputs is it failing on, and what is it returning? – John Kugelman Jul 04 '21 at 04:24
  • It really does not seem like your code is handling the case where an array element is possibly `0` correctly. – Nathan Pierson Jul 04 '21 at 04:26
  • Hint: try to test if the array like that `arr[5]={0,2,2,1,1};` – Abanoub Asaad Jul 04 '21 at 04:39
  • 2
    learn how to debug: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/q/25385173/995714), [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) – phuclv Jul 04 '21 at 05:08
  • @SAFA, A different solution that uses "STL unordered_map" would have a better time complexity that is O(N). Your solution has the time complexity of O(N*log(N)). – Job_September_2020 Jul 04 '21 at 05:19

5 Answers5

1

In every iteration your i is increasing, so this condition which you have written in your for loop:

arr[i]!=index

Here, let's say if the input array has duplicate elements, then for 2 consecutive values of i you will get the same value in arr[i]. In the first comparison, this condition will hold false, so you go to the else part and increment the index value. In the next iteration, your condition arr[i]!=index is always going to be true, as arr[i] is still the same but the index is increased. Thus your program will break from the for loop and the index value is getting returned. That is where it's failing.

So, it will always fail whenever you have duplicate positive elements in your input array. Except for the case when the largest item in the array is the only duplicate in input.

Amit Upadhyay
  • 7,179
  • 4
  • 43
  • 57
1

Here's one hint:

    for(int i=0;i<n;i++){
        if(arr[i]>0){
            if(arr[i]!=index){
                a=index;
                break;
            }
            else{
                index++;
            }
        }
    }

imagine your sorted array is [-10, -5, 0, 1, 2, 3, 4, 5]

When i==3. arr[3] is equal 1, which is the first number you want to evaluate against index. But index will be equal to 3, not 1 as you might have intended.

And as others have pointed out - duplicate numbers in the array are not handled either.

Second hint:

What if I told you... that there was a way to solve this problem without having to sort the input array at all? What if you had an allocated an array of bools of length N to work with....

selbie
  • 100,020
  • 15
  • 103
  • 173
  • An allocated array of bools of length `n` doesn't seem ideal, considering the O(1) additional memory requirement. There's also an O(n) time complexity requirement, though, which rules out any solution that requires sorting the input first. Just maintain a `smallestPositiveNotSeen` variable, I'd say – Nathan Pierson Jul 04 '21 at 05:00
  • only if that if condition which is arr[i]>0 gets true it will go inside right,then how can index becomes 3? – SAFA Jul 04 '21 at 05:02
  • No just tracking `smallestPositiveNotSeen` doesn't work when the array is unsorted. Hm. Not super obvious what algorithm to use. – Nathan Pierson Jul 04 '21 at 05:07
1

You should only increase index if arr[i] == index or else you'll get the wrong result for arrays with duplicates, like {1,2,3,4,5,5,6,7}.

int missingNumber(int arr[], int n) { 
    std::sort(arr,arr + n);

    int index=1;
    int a;
    for(int i=0; i < n; i++) { 
        if(arr[i] > 0) {
            if(arr[i] == index) {       // equal, step
                ++index;
            } else if(arr[i] > index) { // greater, we found the missing one
                a=index;
                break;
            }                           // else, arr[i] == index - 1, don't step
        }
    }
    return index;
}

You are missing a great opportunity to use the sorted array though. Since you're only interested in positive numbers, you can use std::upper_bound to find the first positive number. This search is done very efficiently and it also means that you don't have to check if(arr[i] > 0) in every iteration of your loop.

Example:

int missingNumber(int arr[], int n) { 
    int* end = arr + n;

    std::sort(arr, end);        
    int* it = std::upper_bound(arr, end, 0); // find the first number greater than 0

    int expected = 1;
    
    while(it != end && *it <= expected) {
        if(*it == expected) ++expected;
        ++it;
    }
    
    return expected;
}

Alternatively, std::partition the array to put the positve numbers first in the array even before you sort it. That means that you'll not waste time sorting non-positive numbers.

int missingNumber(int arr[], int n) { 
    int* end = arr + n;

    end = std::partition(arr, end, [](int x){ return x > 0; });
    std::sort(arr, end);        

    int expected = 1;
    
    for(int* it = arr; it != end && *it <= expected; ++it) {
        if(*it == expected) ++expected;
    }

    return expected;
}
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
0

You can try using a counting array and then walk the array until you come to an empty space.

int main() {
    int N;
    cin >> N;
    int num; // set to zero b/c zero is out lowest possible number
    vector<int> numbers;
    while (cin >> num) {
        numbers.push_back(num); 
    }

   //create a counting array to add a 1 to all the positions that exist
    int * cA = new int[10000] {0};
    
    for (int i = 0; i < N; i++) {
        if (numbers[i] >= 0) {
            cA[numbers[i]]++;
        }
    }
    
    for (int i = 1; i < 10000; i++) {
        if (cA[i] == 0) {
            num = i;
            break;
        }
    }
    cout << num;
    
    delete []cA;
    
    return 0;
}
h0tst3w
  • 114
  • 4
  • Always get a little weirded out when I see something like `int * cA = new int[10000] {0};` in a function already using `vector` Side note: If N can be 1000000 10000 isn't nearly enough – user4581301 Jul 04 '21 at 05:24
  • Definitely, the vector is to hold values from the STDIN; we don't know how many values we're going to get. The dynamic array is for the counting array, and we dynamically allocate that because the stack might be full. The example I looked at didn't have a max number so I just put 10000 as something. OP can change if they end up using. – h0tst3w Jul 04 '21 at 05:28
  • Apologies for being unclear. Why not use a second `vector` in place of the manually-managed dynamic array? – user4581301 Jul 04 '21 at 05:38
  • We don't know where the values are going to be, so we use an array so we can take advantage of the constant time insert. Seeing as though our lowest value is 0, we just have to count. If I used a vector, I would have to push_back each time which is expensive (resizing and all that). – h0tst3w Jul 04 '21 at 05:45
  • Not necessarily. `vector cA(1000000);` takes care of all of the problems: random access, RAII memory management, and sufficient size for all possible user inputs. The same trick can be used with `numbers`: `vector numbers(N); size_t num = 0; while (cin >> numbers[num++] )` – user4581301 Jul 04 '21 at 05:50
  • That said, a combination of the approaches may be faster: `vector numbers; numbers.reserve(N); int num; while (cin >> num) numbers.push_back(num);` Compilers these days a re pretty smart so it's hard to say which will be better. – user4581301 Jul 04 '21 at 05:56
  • The almighty compilers and their being smarter than us save the day... again. – h0tst3w Jul 04 '21 at 07:00
0

How Code Works : first get element count and add all items into Vector by Loop,with second loop going to 1000 i check from 1 to 1000 if any of 1,2,3,4,... is not in the vector i print missing,i do this with bool variable res,if any of loop counter starting from 1 to 1000 is in the vector res variable is set to True otherwise False.be careful in each run of For Loop from 1 to 1000 you should set res=False

#include <iostream>
#include <vector>
using namespace std;
//Programmer : Salar Ashgi
int main()
{
    vector<int> v;
    int k=0;
    cout<<"Enter array count ?\n";
    cin>>k;
    int n;
    for(int i=0;i<k;i++)
    {
        cout<<"Enter num "<<i+1<<" : ";
        cin>>n;
        v.push_back(n);
    }
bool res=false;
for(int i=1;i<=1000;i++)
{
    res=false;
    for(int j=0;j<k;j++)
    {
        if(v[j]==i)
        {
            res=true;
            break;
        }
        
    }
    if(!res)
    {
        cout<<i<<" is missing !";
        break;
    }
        
    
}
    
}
Salar Ashgi
  • 128
  • 2
  • 13