0

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;
}
underscore_d
  • 6,309
  • 3
  • 38
  • 64
Dragan Itm Smoljan
  • 127
  • 2
  • 10
  • 9
    `qsort` doesn't work properly with `std::string`, try `std::sort` instead. – Bo Persson Feb 15 '16 at 14:30
  • 2
    The `qsort()` function uses byte-by-byte copying to move data. That may not work properly for strings. – Jonathan Leffler Feb 15 '16 at 14:32
  • I used `g++ (Ubuntu 4.8.4-2ubuntu1~14.04.1) 4.8.4` And output was: `dragan 4 matija 3 zdendka 2 darko 1`. I guess it's some kind of implementation detail, or that string is not designed to be used with `qsort`. Also probably you are using older compiler. – 8bra1nz Feb 15 '16 at 14:35
  • 3
    "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 Feb 15 '16 at 14:38
  • 2
    Which book is it? We need to compile a list of books to avoid. – n. m. could be an AI Feb 15 '16 at 14:52
  • its a croatian C++ book, so what is incorect, instead of using "array" structure to form a list of Persons, they are using normal arrays which dont support functions on it? – Dragan Itm Smoljan Feb 15 '16 at 15:02
  • 1
    'so what is incorect': It's a bizarre mishmash of C++ (`string`, `iostream`s), 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 Feb 15 '16 at 15:08
  • It's nothing to do with the type of container and everything to do with the the types of objects and their members. But that is far from the main problem. Graham is right on: the book is recommending a hybrid abomination that combines several languages' (if we consider C++ pre-11 to be a different, vastly inferior language - as we should - controversial!) features and in doing so incurs none of those languages' benefits but a sizeable serving of their drawbacks. – underscore_d Feb 15 '16 at 15:18

1 Answers1

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;
}
David Haim
  • 25,446
  • 3
  • 44
  • 78
  • 1
    On systems using the [short string optimization](http://stackoverflow.com/questions/10315041/meaning-of-acronym-sso-in-the-context-of-stdstring) `qsort` just happens to be able to sort short strings (by chance, and as one possible outcome of UB). – Bo Persson Feb 15 '16 at 14:59
  • 1
    `#include ` – David Haim Feb 15 '16 at 15:02
  • cant still find array, and cant deduce && auto...nvm i will skip this troubled code =/ – Dragan Itm Smoljan Feb 15 '16 at 15:17
  • The first part of the answer just encourages usage of `qsort` when it should be avoided like the plague for the OP's situation. – PaulMcKenzie Feb 15 '16 at 15:20
  • @DraganItmSmoljan What compiler and settings are you using? – underscore_d Feb 15 '16 at 15:22
  • netbeans 8.1 with c++ 11 standard and still with it get many other errors too many initializers for std::array for example... – Dragan Itm Smoljan Feb 15 '16 at 15:26
  • probably the `array` needs another set of braces around its elements - for aggregate initialisation of the internal raw array. – underscore_d Feb 15 '16 at 15:30
  • Yes, the `array` initializer requires three braces. Also, the explicit inclusion of copy and move constructors deletes the copy and move assignment operators. (What is the point of all those defaults, exactly?). The code lacks #include directives, and uses `std::` inconsistently. And finally, the "C++ way" would include defining `operator<`, probably implemented with the help of `std::tie`. – rici Feb 15 '16 at 15:41
  • While I appreciate the intention, pasting an untested snippet/suggestion without qualification is counterproductive. Most of the comments on this are simply because it wasn't complete or properly tested before posting. – underscore_d Feb 15 '16 at 15:59
  • @PaulMcKenzie std::array people {{{"darko", 1},{"zdendka", 2},{"matija", 3},{"dragan", 4}}}; why 3 braces? im sorry if it sound trivial, but im new to c++. – Dragan Itm Smoljan Feb 16 '16 at 09:43