Click here to Skip to main content
15,867,488 members
Please Sign up or sign in to vote.
1.00/5 (1 vote)
See more:
in this function it always display an error that a and b are initialized variables, but i think there is more problems in the code

What I have tried:

#include <iostream>
using namespace std;
void PosNeg(int* arr, int n, int* a, int* b, int& cn, int& cp)
{
    cn = 0, cp = 0;
    for (int i = 0; i < n; i++) {
        if (arr[i] >= 0) {
            cp++;
        }
        else {
            cn++;
        }
    }
    a = new int[cn];
    b = new int[cp];
    for (int i = 0; i < n; i++) {
        if (arr[i] < 0) {
            arr[i] = a[i];
        }
        else {
            arr[i] = b[i];
        }

    }
    delete[]a;
    delete[]b;
}
int main() {
    int n, cn, cp;
    int* a;
        int* b;
    cout << "Enter number of elements:" << endl;
    cin >> n;
    int* A = new int[n];
    cout << "Enter the elements:" << endl;
    for (int i = 0; i < n; i++) {
        cin >> A[i];
    }
  
    PosNeg(A, n, a, b, cn, cp);
    for (int i = 0; i < cn; i++) {
        cout << a[i] << " ";
    }
    for (int i = 0; i < cp; i++) {
        cout << b[i] << " ";
    }
}
Posted
Updated 30-Mar-20 22:01pm

This fixes that (ugly, as you've already been told) C++ code
C++
#include <iostream>
using namespace std;

void PosNeg(int* arr, int n, int* &a, int* &b, int& cn, int& cp)
{        
    cn = 0, cp = 0; 
    for (int i = 0; i < n; i++) {
        if (arr[i] >= 0) {
            cp++;
        }
        else {
            cn++;
        }
    } 
    a = new int[cp];
    b = new int[cn];
    int in = 0;
    int ip = 0;
    for (int i = 0; i < n; i++) {
        if (arr[i] >= 0) {
            a[ip++] = arr[i];
        }
        else {
            b[in++] = arr[i];
        }
    }
}
int main() {
    int n, cn, cp;
    int* a;
    int* b;

    cout << "Enter number of elements:" << endl;
    cin >> n;

    int* A = new int[n];

    cout << "Enter the elements:" << endl;
    for (int i = 0; i < n; i++) {
        cin >> A[i];
    }

    PosNeg(A, n, a, b, cn, cp);
    cout << "non-negative: ";
    for (int i = 0; i < cp; i++) {
        cout << a[i] << " ";
    }
    cout << "\nnegative: ";
    for (int i = 0; i < cn; i++) {
        cout << b[i] << " ";
    }
    cout << "\n";
    delete [] a;
    delete [] b;
}
 
Share this answer
 
Solution 1 does a good job of fixing all your errors. However, most of these errors wouldn't have been possible if you had used C++ features instead. Here's a different solution that avoids most of the pitfalls you've fallen into:

C++
#include <iostream>
#include <vector>

using namespace std;

pair<vector<int>,vector<int>> PosNeg(vector<int> input_values)
{
    std::vector<int> positives;
    std::vector<int> negatives;
    for (size_t i = 0; i < input_values.size(); ++i)
        if (input_values[i] < 0)
            negatives.push_back(input_values[i]);
        else
            positives.push_back(input_values[i]);
    return make_pair(negatives, positives);
}

int main()
{
    // read values
    std::vector<int> values;
    int n;
    cout << "Enter numbers. Enter any character to finish." << endl;
    do
    {
        cin >> n;
        if (cin.fail())
            // could not read (or convert to) integer value --> exit loop
            break;
        else
            // store this value
            values.push_back(n);
    } while (cin.good());

    // separate
    auto results = PosNeg(values);

    // write output
    cout << "negative values:" << endl;
    for (size_t i = 0; i < results.first.size(); ++i)
        cout << results.first[i] << " ";
    cout << endl;
    cout << "positive values:" << endl;
    for (size_t i = 0; i < results.second.size(); ++i)
        cout << results.second[i] << " ";
    cout << endl;

    return 0;
}


Improvements compared to the things you did:
1. no need to allocate any array by using std::vector
--> no way to mess up your array deallocation
2. no need to keep track of array sizes by using std::vector
--> no need to determine required array sizes up front
--> no need to pass array sizes into and out of the function
3. can append additional values using vector::push_back()
--> no need to keep track of array index values
--> no way to use incorrect index (yours went out of bounds because you used the input array index for the output arrays as well!)
4. can return the two result arrays as function return by using std::pair
--> no need to specify (and initialize) result arrays before calling the function
--> no need to extend function argument list unneccesarily with output values
--> no way to mess up passing output values by value instead of by reference (you did that)

There are still more things you could do, e. g. using range syntax instead of index values, but that doesn't really improve the program.

I hope this gives you an idea of the usefulness of C++ types such as vector and pair, and gives you an incentive to read up on C++ features so you can use them properly in your future programs.
 
Share this answer
 
Comments
CPallini 31-Mar-20 5:42am    
Have my 5.
"as you've been told" was not casual, in my answer:
https://www.codeproject.com/Questions/5262520/Cplusplus-function-to-reorder-even-elements-in-an
Stefan_Lang 31-Mar-20 5:46am    
Thanks. I had 5ed you as well - why would I refer to your solution otherwise ;-)
Start by looking at what happens to the variable a in the function main. It declared as a pointer to an integer and it is not initialized. It is passed to the function PosNeg. To review, it is an uninitialized pointer passed by value to a function. Once in the function, it is assigned a newly allocated array of the size cn or the number of negative values. To review again, the value a on the stack is assigned to a poniter to a newly allocated array of integers. Then, the array arr is assigned values from a that have never been set. Finally, the array a is deleted. How can you possibly expect there to be any data in a?

You have so many logic errors that you really need to examine every line of code and determine if it is what you really want to do there.
 
Share this answer
 

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900