Dynamic Arrays and user input c++

Question

I'm writing a program for my C++ class that takes the user input for the size of a int and char array, fills the arrays with random values (numbers 0-100, letters A-Z) then sorts, reverses, and displays both arrays.

For the most part the program works and I understand the logic I used here, but...

After running and debugging the code multiple times. I noticed that when the arrays were being filled with values, the first element, even though it was actually being given a value, it would not print the assigned value it was given in ascending order, but would in a descending order? I don't understand this at all.

NOTE: I have to use template functions for the sorting, reversing and display of the arrays.

template <class T> 
void sort(T *arr, int a) {
    T temp;
    for (int i = 0; i < a; i++) {
        for (int j = a; j > 0; j--) {
            if (arr[i] > arr[j]) {
                temp = arr[i];
                arr[i] = arr[j];
                arr[j] = temp;
            }
        }
    }
 }

template <class T>
void reverse(T *arr, int a) {
    T temp;
    for (int i = 0; i < a / 2; i++) {
        temp = arr[i];
        arr[i] = arr[a - i];
        arr[a - i] = temp;
    }
}

template <class T>
void display(T *arr, int a) {
    for (int i = 0; i < a; i++) {
        cout << arr[i] << ", ";
    }
    cout << endl;
}

template<class T>
void save(T *arr, int a) {
    sort(arr, a);
    display(arr, a);
    reverse(arr, a);
    display(arr, a);
}

int main() {

    int x, y;

    cout << "Please enter a number for an array of data type \"int\"" << endl;
    cin >> x;
    cout << "Please enter a number for an array of data type \"char\"" << endl;
    cin >> y;

    int *arr1 = new int[x];
    char *arr2 = new char[y];

    for (int i = 0; i < x; i++) 
        cout << (arr1[i] = rand() % 100 + 1);

    srand(time(nullptr));
    for (int i = 0; i < y; i++)
        cout << (arr2[i] = rand() % 26 + 65);


    system("cls");

    save(arr1, x);
    save(arr2, y);

    delete[]arr1;
    delete[]arr2;

    system("pause");
    return 0;
}

since this is just a short homework it's not too drastic but for the future, avoid `using namespace std;` `system("cls")` `system("pause")` — Folling, Feb 08 '18 at 17:33
@Folling My Prof makes us use "using namespace std;" why is it a bad idea? — eM3e, Feb 08 '18 at 17:37
@eM3e: https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice — Arnav Borborah, Feb 08 '18 at 17:40
If you want a dynamic array, use [std::vector](http://en.cppreference.com/w/cpp/container/vector). — Jesper Juhl, Feb 08 '18 at 17:47
Why is there a `srand(time(null))` after a call to `rand` and none before ? Also, that sort (that's supposed to be a sort, right ?) is a bit weird, don't you mean `for(int j = i + 1 ; j < a ; ++j)` ? — Caninonos, Feb 08 '18 at 17:53
Btw, I feel like it's more idiomatic to pass a `begin` and `past-the-end` pointer(/iterator) to represent a [begin; end) range (ie begin included, end excluded) than a `begin` pointer and the range's size. Interestingly, it's harder to make those accidental off-by-one errors when you manipulate such pairs of pointers/iterators (although that's not why I deem them preferrable) — Caninonos, Feb 08 '18 at 18:10
_My Prof makes us use "using namespace std;"_ I don't believe this — Killzone Kid, Feb 08 '18 at 18:13
namespace std is the namespace in which all aspects of the c++ std are implemented. Imagine it like a shoebox. If you want to get a shoe out of a shoebox you have to specify which shoebox you want to access. So you say `std::cout`, meaning the object cout in the std shoebox. If you declare `using namespace std` it's as if you just took that giganteous shoebox and spilled it on the floor. You'd have a lot of trouble coordinating which shoe you want to use. — Folling, Feb 08 '18 at 19:24

R Sahu · Answer 1 · 2018-02-08T17:56:15.887

1

You have a off-be-one error in couple of places.

    for (int j = a; j > 0; j--) {

is incorrect. a is an invalid index for the array. Change that line to use j = a-1:

    for (int j = a-1; j > 0; j--) {

You have a similar, off-by-one, error in reverse. Instead of

    arr[i] = arr[a - i];
    arr[a - i] = temp;

you need to use:

    arr[i] = arr[a - i - 1];
    arr[a - i - 1] = temp;

Your implementation of sort is not correct. I don't want to get into the algorithmic details here but changing the order of the values used for j seems to fix the problem.

for (int i = 0; i < a; i++) {
    for (int j = i+1 ; j < a ; j++) {
       // The swapping code.
    }
}

edited Feb 08 '18 at 17:56

answered Feb 08 '18 at 17:33

R Sahu

204,454
14
159
270

2

@eM3e, I didn't understand that comment. – R Sahu Feb 08 '18 at 17:37
@ R Sahu my problem original was that the first element would not print in ascending order but would in descending order. Now it doesn't print the first element in descending order but does in ascending order... – eM3e Feb 08 '18 at 17:40

JoJo Brandon Coffman · Accepted Answer · 2018-02-08T17:56:26.110

You are using the complete length here:

save(arr1, x);
save(arr2, y);

So in reverse

arr[i] = arr[a - i]; arr[a - i] = temp;

you need to -1 on the length or you'll get an invalid index when i == 0

arr[i] = arr[a - 1 - i]; arr[a - 1 - i] = temp;

Like R Sahu says, in sort

for (int j = a; j > 0; j--) {

you need to -1 because a is the length which will be an invalid index.

for (int j = a-1; j > 0; j--) {

As a side note, you can declare Temp t inside of the for loop in reverse and inside of the if in sort because it is only used in those scopes.

EDIT: Also I overlooked, in sort you need to change

j>0

to

j >= 0

that way you access the first element of the array as well.

Thank you I understand where my logic was wrong now. – eM3e Feb 08 '18 at 18:00 — eM3e, Feb 08 '18 at 18:00

score 0 · Answer 3 · answered Feb 08 '18 at 18:29

You are using bubble sort that is O(n^2) time complexity. Consider using faster algorithm. If you don't want to implement it on your own, use sort() function. It's complexity is about O(n log n), which is very good.

http://www.cplusplus.com/reference/algorithm/sort/

#include <iostream>
#include <algorithm>

using namespace std;

bool comp(int i1, int i2) { // comp function is to compare two integers
    return i1 < i2;
}

int main() {

    int x[30];
    int n;
    cin >> n;
    for (int i = 0; i < n; i++) {
        cin >> x[i];
    }

    sort(x, x + n, comp); // if you don't provide comp function ( sort(x, x+n)     ), sort() function will use '<' operator

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

    return 0;
}

Dynamic Arrays and user input c++

3 Answers3