Problem with user defined function for testing whether elements of an array are in order

Question

i am trying to write a function that tests whether or not all elements of a given array are ordered from least to greatest, i.e., a[0] <= a[1] <= a[2] ... is true.

i am testing this using a for loop with the condition a[i] > a[i + 1]. the function is supposed to return the index value of the element for which this condition is not true. otherwise, it's supposed to return -1.

when i run this function on test2[], it should return 5 because 4 is smaller than 5, and the index value of 4 in the test2[] array is 5. but i keep getting -1, which incorrectly indicates that the array's elements are ordered from least to greatest.

any advice on how to fix this? i've been trying for a long time.

thank you in advance for any insight. my program is pasted below.

#include <iostream>

using namespace std;

int outOfOrder(double a[], int size);

int main() {


  double test2[] = {1, 2, 3, 4, 5, 4, 8};
  int size = 7;

  outOfOrder(test2, size);

  return 0;

}

int outOfOrder(double a[], int size){

  for (int i = 0; i < size - 1; i++){

    if (a[i] > a[i + 1]){

      cout << i + 1;
      return (i + 1);

    } else {

      cout << -1;
      return -1;

    }

  }

  return 0;

}

Your program doesn't sent test2[] to outOfOrder. It only sends data. — JohnFilleau
– JohnFilleau, Commented Mar 16, 2020 at 21:42
@John, thank you for pointing that out. i've fixed the error but i'm still having the same problem. :( — qwerty
– qwerty, Commented Mar 16, 2020 at 21:46
please dont change the question/code substantially after you got answers, or dont fix errors that you got informed about in comments. I first rolled back your edit because the answer was then wrong. Answer was edited, then I rolled back my rollback, now its consistent again, but it does cause confusion, please dont do it — 463035818_is_not_an_ai
– 463035818_is_not_an_ai, Commented Mar 16, 2020 at 21:49
no worries, I completely understand the reflex to fix your code. You just have to understand that fixes should go to answers while questions are for the broken code ;) — 463035818_is_not_an_ai
– 463035818_is_not_an_ai, Commented Mar 16, 2020 at 22:12

Remy Lebeau · Accepted Answer · 2020-03-16 21:49:55Z

2

Your cout statements do not belong inside the outOfOrder() function. You should handle that output inside of your main() function instead.

In outOfOrder(), the return -1 statement needs to be moved outside of the loop. You are breaking your loop after the 1st iteration regardless of the contents of the array passed in.

Try this:

int main() {
  double test2[] = {1, 2, 3, 4, 5, 4, 8};
  int size = 7;
  cout << outOfOrder(test2, size);
  return 0;
}

int outOfOrder(double a[], int size){
  for (int i = 0; i < size - 1; i++){
    if (a[i] > a[i + 1]){
      return (i + 1);
    }
  }
  return -1;
}

edited Mar 16, 2020 at 21:49

answered Mar 16, 2020 at 21:42

Remy Lebeau

609k36 gold badges516 silver badges875 bronze badges

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

Comments

Vlad from Moscow · Accepted Answer · 2020-03-16 22:17:16Z

The function outOfOrder is incorrect.

int outOfOrder(double a[], int size){

  for (int i = 0; i < size - 1; i++){

    if (a[i] > a[i + 1]){

      return (i + 1);

    } else {

      return -1;

    }

  }

  return 0;

}

It returns -1 as soon as the the next element is not less than the current element. Also it has unclear third return value 0.

It seems you mean the following

size_t outOfOrder( const double a[], size_t size )
{
    size_t i = 0;

    if ( size != 0 )
    {
        while ( ++i < size && !( a[i] < a[i-1] ) );
    }

    return i == size ? -1 : i;
}

That is the first parameter shall have the qualifier const because the array is not being changed in the function. And the second parameter shall have the type size_t.

Usually such a function or algorithm returns the value of the array size that is the value of the second parameter size in the case when all elements of the array satisfy the given condition.

Also do not use magic numbers like 7 in this declaration

int size = 7;

You could calculate the number of elements at least like

const size_t size = sizeof( test2 ) / sizeof( *test2 );

Pay attention to that there is the standard algorithm std::is_sorted_until declared in the header <algorithm>

Here is a demonstrative program that shows the both approaches.

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

size_t outOfOrder( const double a[], size_t size )
{
    size_t i = 0;

    if ( size != 0 )
    {
        while ( ++i < size && !( a[i] < a[i-1] ) );
    }

    return i == size ? -1 : i;
}

int main() 
{
    double test2[] = { 1, 2, 3, 4, 5, 4, 8 };
    const size_t N = sizeof( test2 ) / sizeof( *test2 );

    auto pos = outOfOrder( test2, N );

    std::cout << "pos = " << pos << " test2[" << pos << "] = " << test2[pos] << '\n';

    auto it = std::is_sorted_until( std::begin( test2 ), std::end( test2 ) );

    pos = std::distance( std::begin( test2 ), it );

    std::cout << "pos = " << pos << " test2[" << pos << "] = " << test2[pos] << '\n';

    return 0;
}

The program output is

pos = 5 test2[5] = 4
pos = 5 test2[5] = 4

Collectives™ on Stack Overflow

Problem with user defined function for testing whether elements of an array are in order

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related