2

Beginner in C++ here and learning arrays. The program below is supposed to separate positive and negative numbers in an array. However, it is returning random numbers in both the splitPos and splitNeg functions.

Could someone ever so kindly advice and show me what is incorrect in the functions and what can be done to omit these random digits so that only positive and negative digits are returned respectively by the program for each function/loop? I am obviously not seeing and/or understanding what is incorrect.

Thank you so very much for your help and time in advance!!!

#include <iostream>

using namespace std;

//function prototypes
int splitNeg(int[], int[], int);
int splitPos(int[], int[], int);
void displayArr(int[], int);

int main()
{
    const int SIZE = 20;
    int usedPos, usedNeg;
    int origArr[SIZE] = { 4, -7, 12, 6, 8, -3, 30, 7, -20, -13, 17, 6, 31, -4, 3, 19, 15, -9, 12, -18 };
    int posArray[SIZE];
    int negArray[SIZE];


    usedPos = splitPos(origArr, posArray, SIZE);
    usedNeg = splitNeg(origArr, negArray, SIZE);


    cout << "Positive Values: " << endl;
    displayArr(posArray, usedPos);
    cout << endl;
    cout << "Negative Values: " << endl;
    displayArr(negArray, usedNeg);

    return 0;
}

int splitPos(int origArr[], int posArray[], int SIZE)
{
    int j = 0;

        for (int i = 0; i < SIZE; i++ && j++)
        {

            if (origArr[i] >= 0)
                posArray[j] = origArr[i];

        }


            return j;

}

int splitNeg(int origArr[], int negArray[], int SIZE)
{
    int k = 0;

    for (int i = 0; i < SIZE; i++ && k++)
    {
        if (origArr[i] < 0)
            negArray[k] = origArr[i];

    }

    return k;
}

void displayArr(int newArray[], int used)
{
    for (int i = 0; i < used; i++)
        cout << newArray[i] << endl;
    return;

}
3
  • i++ && j++ is bad because && short circuits. In such cases you should use a comma operator instead: i++, j++ Commented Nov 6, 2016 at 1:35
  • Well-posed question. Commented Nov 6, 2016 at 1:41
  • Look, no loops! Commented Nov 6, 2016 at 1:55

5 Answers 5

2

If you change your for-loops a bit:

int splitPos(int origArr[], int posArray[], int SIZE)
{
    int j = 0;

        for (int i = 0; i < SIZE; i++)
        {

            if (origArr[i] >= 0)
                posArray[j++] = origArr[i];

        }


            return j;

}

int splitNeg(int origArr[], int negArray[], int SIZE)
{
    int k = 0;

    for (int i = 0; i < SIZE; i++)
    {
        if (origArr[i] < 0)
            negArray[k++] = origArr[i];

    }

    return k;
}

you will get the result you desire.

The counter variables of your target arrays only get increased if you find a value that matches the criterion of being less (or greater) than 0.

I honestly do not understand what you tried to achieve with a ... hmmm.. "combined" increase like i++ && j++, this goes into short circuit evaluation.

Sign up to request clarification or add additional context in comments.

Comments

2

j and k must be incremented only when the correct value is copied to posArray and negArray, by definition.

If the value is the wrong sign, j and k, obviously, should remain unchanged, since the number of values with the right sign, in the corresponding output array, does not change on this iteration.

This is not what the code is doing. It is incrementing them on every iteration of the loop, except for the one where i is 0.

Comments

1

There is a standard algorithm made just for that. It is named std::partition.

Your code with that algorithm will look like this:

struct SplitPosAndNegResult {
    std::vector<int> negatives;
    std::vector<int> positives;
};

auto splitPosAndNeg(std::array<int, SIZE> orig) {
    // Here `it` is the first element of the second partition (positives)
    auto it = std::partition(orig.begin(), orig.end(), [](int i){ return i < 0; });
    
    return SplitPosAndNegResult{
        std::vector<int>(orig.begin(), it), // negative numbers
        std::vector<int>(it, orig.end()) // positive numbers
    };
}

Then, use it like that:

int main () {
    auto result = splitPosAndNeg({ 4, -7, 12,  6, 8, -3, 30,  7, -20, -13,
                                   17,  6, 31, -4, 3, 19, 15, -9,  12, -18});

    for (int n : result.positives) {
        std::cout << n << ' ';
    }
    std::cout << std::endl;

    for (int n : result.negatives) {
        std::cout << n << ' ';
    }
    std::cout << std::endl;
}

This program will output this:

7 30 8 17 6 31 6 3 19 15 12 12 4

-18 -7 -9 -4 -13 -3 -20

Here's a live example at Coliru.

Comments

1

All the answers in this post are good, but I'm disappointed that none of them talk about the STL algorithms!

A good c++ programmer must know the language but he have to know also the C++ library.

look the following code:

#include <iostream>
#include <array>
#include <algorithm>
#include <string>

using namespace std;

template<typename T>
void print(const string& desc, T first, T last)
{
    cout << desc;
    for_each(first, last,
      [](const auto& i ) { cout << i << ' ';});
    cout << endl;
}

int main()
{
    array<int, 20> originalArray = { 4, -7, 12, 6, 8, -3, 30, 7, -20, -13, 17, 6, 31, -4, 3, 19, 15, -9, 12, -18 };

    print("original array is ", begin(originalArray), end(originalArray)); 

    auto it = partition(begin(originalArray), end(originalArray),
    [](int n) { return n >= 0; });

    print("now original array is ", begin(originalArray), end(originalArray));
    print("positives are: ", begin(originalArray), it);    
    print("negatives are: ", it, end(originalArray));

    return 0;
}

More generally you want partition your set with a predicate.

Look to my code do you find any if or for? It's impossible make mistakes this way!

The only thing that does matter in the whole code is auto it = partition(begin(originalArray), end(originalArray), [](int n) { return n >= 0; }); that can be read as: partition from start to finish of originalArray elements that are positive.

Comments

-2
#include<iostream>
using namespace std;
int main(){
    int a[10]={9,4,-3,-2,1,-1,5,7,-9,-5};
    int low=0;
    int high=10-1;
    while(low<high){
        while(a[low]>=0){
            low++;
        }
        while(a[high]<=0){
            high--;
        }
        if(low<high){
            int temp=a[low];
        a[low]=a[high];
        a[high]=temp;   
        }

    }
    for(int i=0;i<10;i++){
        cout<<a[i]<<" ";
    }
}

Time Complexity: O(n)

1 Comment

Welcome to Stack Overflow! This question is looking for an explanation, not simply for working code. Your answer provides no insight for the questioner, and may be deleted. Please edit to explain what causes the observed symptoms.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.