Incorrect/buggy sorting of classes containing C++ strings when using qsort()

Question

I got a strange error while compiling this code. Its the same as in the book im studying from, but it seems it gets confused somewhere, because my output is off, ages and names got mixed eg( darko 4, zdendk 3, matija 2, draga(dont know where the "n" is gone) 1) and it says invalid pointer. Help me to understand why this is happening.

#include <iostream>
#include <math.h>
#include <cstdlib>

//#include "Tablica.h"

using namespace std;

/*
 * 
 */

struct Person {

    string name;
    int age;

};

int compare (const void* a, const void* b){

  const Person* p1 = static_cast<const Person*>(a);
  const Person* p2 = static_cast<const Person*>(b);

  if(p1->age != p2->age)
  return p2->age - p1->age;

  return p1->name.compare(p2->name); 


}


int main(int argc, char** argv) {

    Person people[]{{"darko", 1},{"zdendka", 2},{"matija", 3},{"dragan", 4}};

    int numP = sizeof(people) / sizeof(Person);
    qsort(people, numP, sizeof(Person), compare);

    for(Person x:people){

       cout<< x.name <<" "<<x.age<<endl;
    }
    return 0;
}

qsort doesn't work properly with std::string, try std::sort instead. — Bo Persson
– Bo Persson, Commented Feb 15, 2016 at 14:30
The qsort() function uses byte-by-byte copying to move data. That may not work properly for strings. — Jonathan Leffler
– Jonathan Leffler, Commented Feb 15, 2016 at 14:32
"Its the same as in the book im studying from, but it seems it gets confused somewhere" Does your book recommend overlooking C++'s rich set of algorithms tailored to its own containers - and then passing said containers to C algorithms that don't know how to deal with them? If so, please, get a different book. — underscore_d
– underscore_d, Commented Feb 15, 2016 at 14:38
Which book is it? We need to compile a list of books to avoid. — n. m. could be an AI
– n. m. could be an AI, Commented Feb 15, 2016 at 14:52
'so what is incorect': It's a bizarre mishmash of C++ (string, iostreams), C++11 (range based for and initializer lists) and C functions (qsort). Use C++ rich library of functions, classes and templates. So std::array or std::vector rather than the C-array you are using and std::sort instead of qsort and you should be pretty much there. — graham.reeds
– graham.reeds, Commented Feb 15, 2016 at 15:08

David Haim · Accepted Answer · 2016-02-15 14:43:56Z

2

Compiling this program with VC++, the output is

dragan 4
matija 3
zdendka 2
darko 1

which makes perfectly sense, since the lines

 if(p1->age != p2->age)
  return p2->age - p1->age;

always kick in (the age is always different), making the array be sorted according to the ages not the names.

As you told by the comments, it is the best to use real C++ techniques like using std::array and std::sort, not supprisingly the code will be smaller, and will run much faster.

The Code, the C++ way:

class Person {

private:
    string name;
    int age;

public:
    Person() = default;
    Person(const string& name, unsigned int age):
      name(name), age(age) {}
    Person(const Person& rhs) = default;
    Person(Person&& rhs) = default;

    string getName() const {return name;}
    unsigned int getAge() const {return age;}
};

int main(){
   array<Person,4> people{{"darko", 1},{"zdendka", 2},{"matija", 3},{"dragan", 4}};
   sort(people.begin(),people.end(),[](auto a, auto b){  
      return a.getName() < b.getName();
   });

   for (const auto& person : people){
       std::cout<< person.getName() <<" , " << person.getAge()<<"\n";
   } 
   return 0;
}

edited Feb 15, 2016 at 14:43

answered Feb 15, 2016 at 14:36

David Haim

26.7k4 gold badges50 silver badges88 bronze badges

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

10 Comments

Bo Persson Over a year ago

On systems using the short string optimization qsort just happens to be able to sort short strings (by chance, and as one possible outcome of UB).

David Haim Over a year ago

#include <algorithm>

Dragan Itm Smoljan Over a year ago

cant still find array, and cant deduce && auto...nvm i will skip this troubled code =/

PaulMcKenzie Over a year ago

The first part of the answer just encourages usage of qsort when it should be avoided like the plague for the OP's situation.

underscore_d Over a year ago

@DraganItmSmoljan What compiler and settings are you using?

|

Collectives™ on Stack Overflow

Incorrect/buggy sorting of classes containing C++ strings when using qsort()

1 Answer 1

10 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

10 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related