CODEWITHSUNDEEP
c++sortingqsort
user5411115
user5411115

Reputation: 101

getting wrong output while sorting in C++

The following C++ code sorts an array in descending order using qsort:

#include<iostream>
#include<cstdio>
#include <stdlib.h>
using namespace std;

struct player {
  double data;
  int index;
};

struct player A[] = {{0.690277,0}, {0.517857,1}, {0.780762,2}, {0.0416667,3}, {0.0416667,4}};

int compare (const void * a, const void * b)
{
   return ( ((struct player*)b)->data - ((struct player*)a)->data );
}

int main ()
{
  int n;
  qsort (A, 5, sizeof(struct player), compare);
  for (n=0; n<5; n++)
  printf ("data=%lf, index=%d\n", A[n].data, A[n].index);
  return 0;
}

But I am getting output like this:

data=0.517857, index=1
data=0.780762, index=2
data=0.041667, index=3
data=0.041667, index=4
data=0.690277, index=0

Is there anything wrong in the code?

Upvotes: 2

Views: 333

Answers (2)

utnapistim
utnapistim

Reputation: 27365

Consider using this compare instead:

int compare (const void * a, const void * b)
{
    auto x = reinterpret_cast<const player*>(a);
    auto y = reinterpret_cast<const player*>(b);
    if(x->data < y->data)
        return -1;
    if(x->data > y->data)
        return 1;
    return 0;
}

That said, this style of coding is ancient/deprecated/bad practice.

Consider writing similar to this instead:

#include<iostream>
#include <algorithm>

struct player {
  double data;
  int index;

    bool operator < (const player& p) const
    {
        return data < p.data;
    }
};

auto A = std::vector<player>{
    {0.690277,0}, {0.517857,1},
    {0.780762,2}, {0.0416667,3}, {0.0416667,4}
};

int main ()
{
    std::sort(std::begin(A), std::end(A));
    for(const auto& x: A)
        std::cout << "data=" << x.data << ", "
            << "index=" << x.index << "\n";
}

Suggested changes:

  • don't import std names globally
  • don't mix cstdio and iostreams (only include one of them)
  • use std::vector or std::array instead of native array
  • define the sorting order in the interface of the class (bool operator <). (this should also imply that you define the other arithmetic operators - it is good practice and avoids subtle bugs later, but it is not required for this particular implementation to compile and work)
  • use std::sort instead of qsort
  • don't use raw pointers (using them like this is a source for bugs)

Upvotes: 5

weltensturm
weltensturm

Reputation: 2162

In compare, you are subtracting two sub-1 doubles and casting them to an int, the result will in most cases be 0. Instead of subtracting you should compare them and return -1/1.

Upvotes: 8

Related Questions