Search code examples
c++arraysstdlist

using iterator in ostream fails


I am attempting to implement a std::list to replace a linked list in this assignment. I am not allowed to change the declarations and can only change code in the .cpp file. For the most part I am making progress but I am having trouble implementing this

std::ostream& operator<< (std::ostream& out, const Section& section);

namely when I try to create an iterator it fails. I've used the iterator elsewhere in the code so I don't understand why it's failing here, I believe it's because it's private but I'm not sure how to resolve the issue without changing the .h file which was explicitly prohibited:

std::ostream& operator<< (std::ostream& out, const Section& section)
{
  // 1. print the section header
  out << setw(8) << left << section.getCourse()
      << setw(6) << left << section.getCallNumber();
  out << ": " << section.getNumberOfStudents() << " students\n";

  // 2. collect the students, sort, and print
  Student* students = new Student[section.getNumberOfStudents()];
  {
    int i = 0;

    for ( auto pos = section.students.begin();
     pos != section.students.end(); pos++)
      {
    students[i] = pos;
    ++i;
      }
  }

  sort (students, students+section.getNumberOfStudents());

  for (int i = 0; i < section.getNumberOfStudents(); ++i)
    out << "    " << students[i] << "\n";

  out << flush;
  return out;
}

Solution

  • students[i] = pos;
    

    should be changed to

    students[i] = *pos;
    

    because you want to copy the Student the iterator references, not the iterator itself.

    But why a dynamic array of Student rather than a std::vector<Student>? Currently you have a memory leak because you don't delete[] students;

    Edit 1

    Removed.

    Edit 2

    Other than that, all I can see that it wrong is a missing std:: in front of

    sort (students, students+section.getNumberOfStudents());
    

    this is assuming there is no custom sort method being used.

    Edit 3

    Going off the rails here:

    students[i] = *pos;
    

    copies a Student from the list into the dynamic array students. This could be expensive, so here is an alternative:

    First the bits and pieces needed to prove this out: Required includes

    #include <iostream>
    #include <list>
    #include <vector>
    #include <algorithm>
    #include <functional>
    

    a minimal Student class

    class Student
    {
        std::string name; 
    public:
        Student(std::string inname):name(inname)
        {
    
        }
        const std::string & getname() const
        {
            return name;
        }
        friend bool operator<(const Student & a, const Student &b)
        {
            return a.name < b.name;
        }
    };
    

    a minimal Section class

    class Section
    {
    public:
        std::list<Student> students;
    };
    

    a minimal outstream operator

    std::ostream& operator<<(std::ostream& out, const Section& section)
    {
    

    A std::vector instead of an array, and a vector of constant references so we don't have to copy the students.

        std::vector<std::reference_wrapper<const Student>> students;
    

    Store references in the vector. Probably could do a one liner with std::copy and std::back_inserter, but this is getting a bit too much to absorb for one example.

        for (const auto & student: section.students)
        {
            students.push_back(std::ref(student));
        }
    

    Sort the vector

        std::sort(students.begin(), students.end());
    

    print the vector

        for (const auto & student: students)
        {
            out << student.get().getname() << " ";
        }
        return out;
    }
    

    and one main to rule them all and in the darkness bind them

    int main()
    {
        Section s;
    
        s.students.emplace_front("Tom");
        s.students.emplace_front("Dick");
        s.students.emplace_front("Harry");
        std::cout << s;
    }
    

    And all in one easy to cut-n-paste block:

    #include <iostream>
    #include <list>
    #include <vector>
    #include <algorithm>
    #include <functional>
    
    class Student
    {
    public:
        std::string name; // this is me being lazy. name should be private
        Student(std::string inname):name(inname)
        {
    
        }
        const std::string & getname() const
        {
            return name;
        }
        friend bool operator<(const Student & a, const Student &b)
        {
            return a.name < b.name;
        }
    };
    
    class Section
    {
    public:
        std::list<Student> students;
    };
    
    std::ostream& operator<<(std::ostream& out, const Section& section)
    {
        std::vector<std::reference_wrapper<const Student>> students;
    
        // store references in the `vector`.
        for (const auto & student: section.students)
        {
            students.push_back(std::ref(student));
        }
    
         // Sort the `vector`
        std::sort(students.begin(), students.end());
    
        // print the `vector`
        for (const auto & student: students)
        {
            out << student.get().getname() << " ";
        }
        return out;
    }
    
    int main()
    {
        Section s;
    
        s.students.emplace_front("Tom");
        s.students.emplace_front("Dick");
        s.students.emplace_front("Harry");
        std::cout << s;
    }
    

    Or do what Remy suggested and use a std::vector<Student *> and a custom comparator to dereference the pointers for std::sort.