1

I am working through a book called "Think Like a Programmer." Chapter 2 is about solving problems with arrays. There's an exercise at the end of the chapter that asks us to use qsort to sort a an array of structs created earlier in the chapter.

Following the process of the chapter, I created a function, comparator that will be passed to qsort to sort the array, studentArray. My code is...working? However, I'm getting an Address boundary error when running it.

#include <string>
#include <iostream>

using std::cout;
using std::string;

int comparator(const void * voidA, const void * voidB) {
  int * intA = (int *)(voidA);
  int * intB = (int *)(voidB);

  return *intA - *intB;
}

int main() {

  struct student {
    int grade;
    int studentID;
    string name;
  };

  const int ARRAY_SIZE = 10; 

  student studentArray[ARRAY_SIZE] = {
    {87, 10001, "Fred"},
    {28, 10002, "Tom"},
    {100, 10003, "Alistair"},
    {78, 10004, "Sasha"},
    {84, 10005, "Erin"},
    {98, 10006, "Belinda"},
    {75, 10007, "Leslie"},
    {70, 10008, "Candy"},
    {81, 10009, "Aretha"},
    {68, 10010, "Veronica"},
  };

  qsort(studentArray, ARRAY_SIZE, sizeof(student), comparator);

  for (int i = 0; i < ARRAY_SIZE; i++) {
    cout << studentArray[i].grade << "\n";
  }
}

My first assumption was that I messed up the call to qsort with the third parameter. Maybe, I thought, I should only be asking for the size of the first member of the struct (since that's what the first part of the exercise asks us to sort). So, I changed it to:

qsort(studentArray, ARRAY_SIZE, sizeof(student[0]), comparator);

This didn't throw any errors, but it didn't sort the array either. So, all in all, I think I'm just confused about what I'm doing wrong. I don't work with C++ regularly, it's just for the purpose of the book. However, I am really enjoying using it and learning about it, so I would like to understand what causes this problem*. I have searched around online for a while and have seen similar asks, but I can't seem to piece together a solid understanding. I will update this post with any missing information; please just let me know what's needed. I appreciate any and all help with this and hope that it makes sense.

As stated above, I tried a few different things (some of which I'm too embarrassed to mention here).

EDIT: I appreciate the comments and the resources! I'll add one more question to this post: are the concepts taught in the book so closely coupled with the author's C++ implementation that one wouldn't be able to understand what causes this error without a proper understanding of modern C++? Thanks again!

dorraj.
  • 15
  • 4
  • 1
    Sorry to dissapoint you but looks like your book is not teaching current C++ very well. void* is hardly ever used anymore. And if you want to learn C++ you should get a more modern [book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). Or have a look at https://www.learncpp.com/. Have a look at std::vector and [std::sort](https://en.cppreference.com/w/cpp/algorithm/sort). Avoid void* if you can and casts like `(int)v` should be `static_cast(v)` in C++ – Pepijn Kramer Jan 19 '23 at 14:56
  • 3
    Your array contains `student`s, but your comparator assume it is `int`s which causes undefined behavior (UB). Why not use `std::sort` with a strongly typed lambda or functor (instead of the `void*`s) ? – wohlstad Jan 19 '23 at 14:57
  • 1
    [Note that](https://en.cppreference.com/w/cpp/algorithm/qsort) : "Despite the name, C++, C, and POSIX standards do not require this function to be implemented using quicksort or make any complexity or stability guarantees." There is (almost) never a reason to call `qsort` in C++ when you can call `std::sort` – 463035818_is_not_an_ai Jan 19 '23 at 14:59
  • rule of thumb: When you are using a c-style cast (like here `(int *)(voidA);`) then very likely there is something wrong in your code. `std::sort` does not require such `void*` voodoo – 463035818_is_not_an_ai Jan 19 '23 at 15:02
  • 2
    Your struct contains a `std::string`. From what I vaguely recall, `qsort` is not guaranteed to work with non-trivially copyable types. Thus usage of `qsort` for your struct will lead to undefined behavior. – PaulMcKenzie Jan 19 '23 at 15:05
  • @PepijnKramer thank you! I will look into these things. Thanks again for your answer, btw. – dorraj. Jan 19 '23 at 15:07
  • @wohlstad I've seen this used, though during my research I completely neglected to check what year the book was written, as I do with all books (sigh). Yikes, it's now 11-year-old code! – dorraj. Jan 19 '23 at 15:09
  • @463035818_is_not_a_number thank you! @PepijnKramer has posted a good example of ```std::sort``` and I will have a go at that solution today. – dorraj. Jan 19 '23 at 15:10
  • the thing is that even 11 years ago this way of sorting an array was not acceptable, not in real code and especially not in teaching material – 463035818_is_not_an_ai Jan 19 '23 at 15:11
  • @dorraj. -- [Here is the link to qsort and the issues it has with non-POD types](https://stackoverflow.com/questions/6174955/what-kinds-of-types-does-qsort-not-work-for-in-c). I guess the reason is that an implementation of `qsort` does not have to use `=` to exchange elements, which would make it safe. It could call `memcpy` or some other `C` function to exchange elements, thus tearing your objects to shreds while doing the exchange. – PaulMcKenzie Jan 19 '23 at 15:11
  • @PaulMcKenzie I understand. I wish I would've kept my sources up while writing this question so I could add the things I've read to the post. I vaguely remember seeing this somewhere...anyhow, thank you! – dorraj. Jan 19 '23 at 15:12
  • @463035818_is_not_a_number ah, really? Okay. I'm realizing that the types of questions I'm asking may not be sufficient enough to find the answers I need. I really appreciate all of these responses! – dorraj. Jan 19 '23 at 15:14
  • `std::sort` is available in C++98, thats from 25 years ago ;) (and `std::vector` too) – 463035818_is_not_an_ai Jan 19 '23 at 15:15
  • @PaulMcKenzie Again, this has shown me that I'm not thinking hard enough about what I need to know. I was kind of just blindly following the material to an extent. I went through several SO posts and a few pages of documentation, but I didn't consider asking the question the same way as the person in the post you linked. – dorraj. Jan 19 '23 at 15:17
  • be cautios with c++ code that contains `void*`. Valid reasons to use `void*` in C++ have become rare – 463035818_is_not_an_ai Jan 19 '23 at 15:19
  • @dorraj. -- You have to go into your C++ compiler's implementation of `qsort` to see if the exchanging of elements that are out-of-order are compatible with proper C++ copy operations. But again, since `qsort` is written for `C` and probably optimized assuming `C`, the chances are great that your implementation's version of `qsort` doesn't care anything about C++, and attempts to do things as optimial as possible for `C` usage. – PaulMcKenzie Jan 19 '23 at 15:23

2 Answers2

3

For comparison if your book was more current code would look like this - as you see you can use a custom compare function as argument to sort.

#include <algorithm> // for sort.
#include <string>
#include <iostream>
#include <vector>

//using std::cout;
//using std::string;

/*
int comparator(const void* voidA, const void* voidB) {
    int* intA = (int*)(voidA);
    int* intB = (int*)(voidB);

    return *intA - *intB;
}
*/

struct student 
{
    int grade;
    int studentID;
    std::string name;
};

bool compare_students_by_grade(const student& lhs, const student& rhs)
{
    return lhs.grade < rhs.grade;
}

int main() 
{
    std::vector<student> students = // avoid types in your names
    {
      {87, 10001, "Fred"},
      {28, 10002, "Tom"},
      {100, 10003, "Alistair"},
      {78, 10004, "Sasha"},
      {84, 10005, "Erin"},
      {98, 10006, "Belinda"},
      {75, 10007, "Leslie"},
      {70, 10008, "Candy"},
      {81, 10009, "Aretha"},
      {68, 10010, "Veronica"},
    };

    //qsort(studentArray, ARRAY_SIZE, sizeof(student), comparator);
    std::sort(students.begin(), students.end(), compare_students_by_grade); 

    /*
    for (int i = 0; i < ARRAY_SIZE; i++) {
        cout << studentArray[i].grade << "\n";
    }
    */

    // prefer range based for loops, they cannot go out of range
    for (const student& student : students)
    {
        std::cout << student.name << " has grade : " << student.grade << "\n";
    }

    return 0;
}

student& is a reference to an instance of student (kind of like a pointer that must be valid al the time)

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Pepijn Kramer
  • 9,356
  • 2
  • 8
  • 19
  • I don't have enough reputation to mark this as useful, but thank you for this! I appreciate the explanation an the example. I'll have a look into this and the resources shared above to understand more about writing more modern C++. – dorraj. Jan 19 '23 at 15:07
  • 2
    @dorraj. not enough reputation to mark as useful? Its your question, you should be able to accept this answer – 463035818_is_not_an_ai Jan 19 '23 at 15:07
  • If you have any more questions, just ask them. It is quite hard to learn current C++ with all the older code examples still around. – Pepijn Kramer Jan 19 '23 at 15:09
  • 1
    @dorraj. even if you don't have enough rep to **vote**, you can always **accept** (the '✔' mark) an answer to your own question. – wohlstad Jan 19 '23 at 15:14
  • @463035818_is_not_a_number I didn't add it to the question, but I've been awake studying for a while (since midnight my time), so my brain is barely functioning at this point haha. – dorraj. Jan 19 '23 at 15:22
  • @PepijnKramer I do have some more questions, but I want to research first before I do. I'll definitely ask if anything comes up! Thanks – dorraj. Jan 19 '23 at 15:24
  • You do that. :) – Pepijn Kramer Jan 19 '23 at 15:38
1

As mentioned in a comment, your comaprator pretends that the pointers it gets passed are pointers to int but they are pointers to student. Use std::sort:

#include <string>
#include <iostream>
#include <algorithm>
using std::cout;
using std::string;

struct student {
    int grade;
    int studentID;
    string name;
};

bool comparator(const student& a, const student& b) {
    return a.grade < b.grade;
}

int main() {
  const int ARRAY_SIZE = 10; 

  student studentArray[ARRAY_SIZE] = {
    {87, 10001, "Fred"},
    {28, 10002, "Tom"},
    {100, 10003, "Alistair"},
    {78, 10004, "Sasha"},
    {84, 10005, "Erin"},
    {98, 10006, "Belinda"},
    {75, 10007, "Leslie"},
    {70, 10008, "Candy"},
    {81, 10009, "Aretha"},
    {68, 10010, "Veronica"},
  };

  std::sort(std::begin(studentArray),std::end(studentArray),comparator);

  for (int i = 0; i < ARRAY_SIZE; i++) {
    cout << studentArray[i].grade << "\n";
  }
}

Here, the comparator is type safe, using wrong types would result in a compiler error. Also, you need not manually specify the number of elements or their size.

Note, that this answer illustrates how to change your code for more idomatic C++. The code presented here on the other hand is only replacing qsort and minimum changes otherwise.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185