why my C program can't get the two right index?

Question

#include<bits/stdc++.h>
using namespace std;
 
void createArray(int *a,int n){
    for(int i = 0;i < n;i ++){
        int data;cin>>data;
        a[i] = data;
    }
}

void solution(int *a,int n,int toSum){
    sort(a,a+n);
    unordered_map<int,int>exist;
    for(int i = 0;i < n;i ++){
        exist[a[i]] = i;
    }
    for(int i = 0;i < n;i ++){
        int toFind = toSum - a[i];
        if(exist.find(toFind) != exist.end()){
            cout<<i<<" "<<exist[toFind]<<endl;
            break;
        }
    }
}

int main(){
    int n;cin>>n;
    
    while(n != 0){
        int a[n];
        createArray(a,n);
        int n1;cin>>n1;
        solution(a,n,n1);
        cin>>n;
    }
}

Answer 1

Could you please

• describe in details, what your program is supposed to do
• provide inputs that cause a problem for you and also provide what the expected outcome you think should be.

Without this, I'll go with my own assumptions what the problem may be.

I believe what you're trying to do is ask the user to provide an array and a number and the program tries to find two numbers in this array where the sum of those equals to number. The program prints the indices of these numbers.

If you remove the sorting (sort(a,a+n);) your code should return the right indices. However, it will not find all the possible index pairs. For that, you must not break the loop when you find a working solution.

I would also recommend printing to the user what they should do before you ask for input and using better variable names.

All in all, your code could look something like this after some fix-up:

#include <iostream>
#include <unordered_map>

using namespace std;
 
void createArray(int * a, int n)
{
    for(int i = 0; i < n; i++)
    {
        int data;
        cout << "data[" << i << "] = ";
        cin >> data;
        a[i] = data;
    }
}

void solution(int * a, int n, int toSum)
{
    unordered_map<int, int> exist;
    for (int i = 0; i < n; i++)
    {
        exist[a[i]] = i;
    }
    
    cout << "Index pairs successfully summing indexed elements to " << toSum << ":" << endl;
    for (int i = 0; i < n; i++)
    {
        int toFind = toSum - a[i];
        unordered_map<int, int>::const_iterator itemIterator = exist.find(toFind);
        if (itemIterator != exist.end())
        {
            cout << '\t' << i << " " << itemIterator->second << endl;
        }
    }
}

int main()
{
    int arraySize;
    cout << "Array size (0 to quit) = ";
    cin >> arraySize;
    
    while (arraySize != 0)
    {
        int array[arraySize];
        createArray(array, arraySize);
        int sum;
        cout << "sum = ";
        cin >> sum;
        solution(array, arraySize, sum);
        cout << "Array Size (0 to quit) = ";
        cin >> arraySize;
    }
}

Answer 2

There are a few mistakes in your code, and I will explain them.

1.Array Size Declaration: In C++, variable-length arrays (VLAs) are not part of the standard. While some compilers might support them as an extension, it's generally safer to use dynamic memory allocation (e.g., `std::vector`) or a fixed-size array. In your case, you're using a variable-length array `int a[n];`, which might not be supported by your compiler.

2.Using `cin` Inside a Loop: Inside the `main()` function, you're continuously using `cin` without clearing the input buffer. This might cause issues, especially if there are unexpected characters in the input stream.

3.Library Inclusion: Including the entire `<bits/stdc++.h>` might not be necessary. It's better to include only the required headers explicitly.

4.Logic for Finding Indices: The logic in your `solution()` function seems fine, where you're using a hash map to store the indices of elements as keys. However, there might be issues with the actual indices calculation.

#include <iostream>
#include <unordered_map>
#include <algorithm>

using namespace std;

void createArray(int *a, int n) {
     for (int i = 0; i < n; i++) {
         int data;
         cin >> data;
         a[i] = data;
     }
}

void solve(int *a, int n, int toSum) {
     unordered_map<int, int> exist;
     for (int i = 0; i < n; i++) {
         int toFind = toSum - a[i];
         if (exist.find(toFind) != exist.end()) {
             cout << exist[toFind] << " " << i << endl;
             return;
         }
         exists[a[i]] = i;
     }
     cout << "No solution found." << endl;
}

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

     while (n != 0) {
         int *a = new int[n]; // Dynamic memory allocation
         createArray(a, n);
         int n1;
         cin >> n1;
         solution(a, n, n1);
         delete[] a; // Freeing memory
         cin >> n;
     }
     return 0;
}

I've replaced the variable-length array `int a[n];` with dynamic memory allocation using `new int[n];`. Remember to free this memory using `delete[]` after you're done using it.

I removed the sorting step as it is not necessary for finding the indices that sum up to a given value.

I rearranged the logic inside the `solution()` function to first check if the complement exists and then update the map. This ensures that the same element is not used twice.

I've removed the unnecessary inclusion of the `<bits/stdc++.h>` header and included only the necessary standard headers.