Reversing the order of a sort

Question

Hello so I have my program that has to do with a college class schedule and it has a couple sorting functions we will deal with the ascending gpa function and descending gpa function.

This is the descending function and it functions properly:

void classSchedule::downGPA(classSchedule schedule[], int& numElems)
{
    classSchedule temp;
    int end;
    for (end = numElems - 1; end >= 0; end--)
    {
        for (int counter = 0; counter < end; counter++)
        {
            if (schedule[counter].classNumber == 000)
                counter++;
           if (schedule[counter].currentGPA < schedule[counter + 1].currentGPA)
           {

              temp = schedule[counter];
              schedule[counter] = schedule[counter + 1];
              schedule[counter + 1] = temp;
           }
       }
    }
    schedule->outputToConsole(schedule, numElems);

}

This is the ascending function and it displays nothing for some reason:

void classSchedule::upGPA(classSchedule schedule[], int& numElems)
{
    classSchedule temp;
    int end;
    for (end = numElems - 1; end >= 0; end--)
    {
        for (int counter = 0; counter < end; counter++)
         {
            if (schedule[counter].classNumber == 000)
                counter++;
           if (schedule[counter].currentGPA > schedule[counter + 1].currentGPA)
           {

              temp = schedule[counter];
              schedule[counter] = schedule[counter + 1];
              schedule[counter + 1] = temp;
           }
       }
    }
    schedule->outputToConsole(schedule, numElems);
}

I changed the sign and it doesnt display anything can anyone see why?

Edit:

As requested the output function

void classSchedule::outputToConsole(classSchedule currentSchedule[], int numElems)
{
    int i;
    cout << endl << "Dept" << "\t" << "Class Number\t" "Credit Hours" << "\t" << "Name"
         << "\t" << "Room Number" << "\tGPA"
         << endl << "----" << "\t------------" << "\t-------------   ----"
         << "\t-----------" << "\t---";
    for (i = 0; i < numElems; i++)
    {
        if (currentSchedule[i].displayOrNot == "FALSE")
            i++;
        if(currentSchedule[i].currentGPA == -1)
        {
            break;
        }
        cout << endl << currentSchedule[i].classDepartment << "         " << currentSchedule[i].classNumber << "    \t"
             << "      " << currentSchedule[i].creditHours << "    \t"
             << currentSchedule[i].teacherLastName << " " << currentSchedule[i].teacherFirstName
             << "\t" << currentSchedule[i].roomWingAndNumber << "\t" <<      currentSchedule[i].currentGPA;
    }

}

Answer 1

Here is a little helper that makes sorting by some derived value easy:

template<class F>
struct order_by_t {
  F f;
  template<class Lhs, class Rhs>
  bool operator()( Lhs const& lhs, Rhs const& rhs ) const {
    return f(lhs) < f(rhs);
  }
};
template<class F>
order_by_t< F > order_by( F f ) { return {std::move(f)}; }

You call order_by( function ) and it returns a function object that takes values, and orders them by whatever the function returns.

So order_by( function ) returns a function object that takes two things, and tells you if the left one is "less" than the right one according to the order that function tells you.

This is useful because C++'s standard library can be passed ordering function objects and use them for a whole bunch of reasons.

This helper makes writing upGPA pretty short:

void classSchedule::upGPA(classSchedule schedule[], int numElems) {
  classSchedule* start = &schedule[0];
  classSchedule* finish = start+numElems;
  std::sort( start, finish,
    order_by( [](classSchedule const& s){return s.currentGPA;} )
  );
}

order_by( [](classSchedule const& s){return s.currentGPA;} ) is where the magic happens.

I'll unroll it. A simpler way to write it is:

[](classSchedule const& lhs, classSchedule const& rhs){
  return lhs.currentGPA < rhs.currentGPA;
}

but I like my way.

order_by takes a function of one argument, and builds an ordering (a replacement for <).

It takes a function of type A->B (read it as "elements of type A to elements of type B") such that B is ordered (supports <), and produces a function of type (A,A)->bool that is an ordering on As. It does this by taking each argument, mapping it through the A->B map, then comparing the Bs.

So order_by is of type (A->B) -> (A,A) -> bool that returns an ordering based off of the A->B function.

I find this projection -- of a type to an order -- is quite useful. In this case, it may be overkill.

---

An easy implementation of downGPA -- sort up, then reverse:

void classSchedule::downGPA(classSchedule schedule[], int numElems) {
  upGPA(schedule, numElems);
  classSchedule* start = &schedule[0];
  classSchedule* finish = start+numElems;
  std::reverse( start, finish );
}

you could instead use reverse iterators, or use a negated order-by, or whatever. But the above is simple and less bug prone.

The function upGPA and downGPA shouldn't outputToConsole. Do that in a different step. Printing is a different problem than reordering in upward or downward order.

The above uses C++11. With gcc or clang, you may need to pass in a flag to enable support.

If you lack C++11, this will work:

template<class F>
struct order_by_t {
  F f;
  order_by_t(F in):f(in) {}
  order_by_t(order_by_t const& o):f(o.f) {}
  template<class Lhs, class Rhs>
  bool operator()( Lhs const& lhs, Rhs const& rhs ) const {
    return f(lhs) < f(rhs);
  }
};
template<class F>
order_by_t< F > order_by( F f ) { return f; }
int getCurrentGPA( classSchedule const& s ) {
  return s.currentGPA;
}

then:

void classSchedule::upGPA(classSchedule schedule[], int numElems) {
  classSchedule* start = &schedule[0];
  classSchedule* finish = start+numElems;
  std::sort( start, finish,
    order_by( getCurrentGPA )
  );
}

should work.

or even easier:

bool orderByCurrentGPA( classSchedule const& lhs, classSchedule const& rhs ) {
  return lhs.currentGPA < rhs.currentGPA;
}
void classSchedule::upGPA(classSchedule schedule[], int numElems) {
  classSchedule* start = &schedule[0];
  classSchedule* finish = start+numElems;
  std::sort( start, finish,
    orderByCurrentGPA
  );
}

which does the same thing. (I removed order_by here).

Answer 2

You have couple issues with your sorting functions:

• Your loop condition is counter < end and you use [counter+1] as array index, you will go out of array boundary on the first iteration, suggestion change condition to counter < end -1
• Code if (schedule[counter].classNumber == 000) is dangerous and is not clear why you need it. Probably to avoid previous error

Coolprit seems to be this code:

if(currentSchedule[i].currentGPA == -1)
{
    break;
}

When you sort in descending order that record goes to the end and it works fine. But when you sort in ascending that record is in begining and you interrupt your loop. If you wantto skip records with GPA -1 replace break with continue in that code.