Home > OS >  Why doesn't push_back keep working in a loop?
Why doesn't push_back keep working in a loop?

Time:07-06

Completely new to C . Programmed selection sort on 1D array of arbitrary length. Want to allow user to keep inputting integers into console to make an array of desired length, to be subsequently sorted.

Can only seem to make arrays of length 2 using a while loop for adding elements. Code and example of erroneous result when inputting 6, 2, 3, and 9 shown below.

Script:

// Preprocessor directives and namespace declaration
#include <iostream>
#include <vector>
using namespace std;

// Function
void SelectionSort(int *arr, int len)
{
    // Loop through index j in arr
    for (int j = 0; j < len; j  ) {
        
        // Assume element j is minimum, and initialise minIndex
        int min = arr[j];
        int minIndex = j;
        
        // Loop through comparisons to determine actual minimum
        // (of elements after and including j)
        for (int i = j; i < len; i  )
        {
            if (min > arr[i])
            {
                min = arr[i];
                minIndex = i;
            }
        }
        
        // Swap minimum with element j
        int temp = arr[j];
        arr[j] = min;
        arr[minIndex] = temp;
    }

    // Display resulting array
        for (int i = 0; i   1 < len; i  )
        {
            cout << arr[i] << ", ";
        }
        cout << arr[len - 1] << endl;
}

// Main
int main()
{
    // Explain program to user
    cout << "Sort 1D array of user-inputted length/contents" << endl;
    cout << "To finish array, enter -999" << endl;
    
    // Initialise dynamic array
    vector<int> vDyn (1);
    vDyn[0] = 0;
    cout << "Enter first element of array: ";
    int firstElement = 0;
    cin >> firstElement;
    vDyn[0] = firstElement;
    
    // Loop to define elements until desired length reached
    bool keepGoing = true;
    while (keepGoing == true)
    {
        cout << "Enter another element: ";
        int newElement = 0;
        cin >> newElement;
        if (newElement != -999)
        {
            vDyn.push_back(newElement);
        } else
        {
            keepGoing = false;
        }
    }
    
    // Convert vector to array (dynamic to static)
    int* v = &vDyn[0];

    // Get array length
    int len = sizeof(v) / sizeof(v[0]);
    
    // Run SelectionSort function
    SelectionSort(v, len);
    
    return 0;
}

Terminal:

Sort 1D array of user-inputted length/contents
To finish array, enter -999
Enter first element of array: 6
Enter another element: 2
Enter another element: 3
Enter another element: 9
Enter another element: -999
2, 6

CodePudding user response:

This declaration

int len = sizeof(v) / sizeof(v[0]);

is equivalent to the declaration

int len = sizeof( int * ) / sizeof( int );

because the variable v is declared like

int* v = &vDyn[0];

The size of a pointer is equal usually to 4 or 8 bytes. So the variable length will have the value either 1 or 2 and does not depend on the number of elements stored in the vector..

Instead you should use for example

size_t len = vDyn.size();

You could declare the function like

void SelectionSort(int *arr, size_t len);

and call it like

SelectionSort( vDyn.data(), vDyn.size() );

Also as in C there is standard function std::swap declared in the header <utility> then instead of this code snippet

    // Swap minimum with element j
    int temp = arr[j];
    arr[j] = min;
    arr[minIndex] = temp;

you could just write

if ( j != minIndex ) std::swap( arr[j], arr[minIndex] );

And the inner for loop could look like

for ( size_t i = j   1; i < len; i  )
                 ^^^^^

In fact your function SelectionSort is a C function. A C function should be more general and use iterators. In this case it could sort arrays along with other containers.

Here is a demonstration program that shows a more general function called for an array based on a vector.

#include <iostream>
#include <vector>
#include <iterator>
#include <algorithm>

template <typename ForwardIterator>
void SelectionSort( ForwardIterator first, ForwardIterator last )
{
    for ( ; first != last;   first )
    {
        auto current_min = first;

        for ( auto next = std::next( first ); next != last;   next )
        {
            if ( *next < *current_min ) current_min = next;
        }

        if ( current_min != first )
        {
             std::iter_swap( current_min, first );
        }
    }
}

int main()
{
    std::vector<int> v = { 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 };

    for ( const auto &item : v )
    {
        std::cout << item << ' ';
    }
    std::cout << '\n';

    SelectionSort( v.data(), v.data()   v.size() );

    for ( const auto &item : v )
    {
        std::cout << item << ' ';
    }
    std::cout << '\n';
} 

The program output is

9 8 7 6 5 4 3 2 1 0 
0 1 2 3 4 5 6 7 8 9 

In general you need also to write an overloaded function that accepts also a comparison function.

CodePudding user response:

// Convert vector to array (dynamic to static)
int* v = &vDyn[0];

This line doesn't convert the array to anything. You merely take address of the first element in the vector.

If you want to take an underlying c-array from std::vector you are supposed to use data property of it.

Also, since the array is decayed into a pointer, it no longer contains data of its size. You should rely on std::vector properties (i.e. std::vector::size) to pass this information forward

  • Related