Add elements to std::list using iterator

[L1bXjE8b]

As the title states, I am trying to add elements to a std::list using an iterator. I have std::list<Student> students; declared in a header file, and I am trying to add students using push_back(). Here are the areas I'm having trouble with:

Section::Section (const Section& sect) {
  list<Student>::const_iterator it;
  for (it = sect.begin(); it != sect.end(); ++it)
    students.push_back ( sect.students[ it ] ); //Problem here
}

Section& Section::operator= (const Section& sect) {
  if (this != &sect) {
  	   students.clear();
  }
    list<Student>::const_iterator it;
    for ( it = sect.begin(); it != sect.end (); ++ it ) {
        students.push_back ( sect.students[ it ] ); //problem
    }

    for ( it = sect.begin(); it != sect.end(); ++it ) {
       addStudent (current->data); //I know this isn't right
    }
  return *this;
}

If std::list<Student>students is being used, what is the best way to add students to the list? Thanks in advance!

[ne555]

> I know this isn't right
¿then why you do that?

> Problem here
¿can you be a little more descriptive?


¿why `addStudent()' only appears in your assignment operator?
¿why do you even need to code a copy constructor and an assignment operator?

[newbieg]

I think you are actually wanting list::insert();
http://www.cplusplus.com/reference/list/list/insert/
Either that or you might be looking for emplace, though my work with lists is limited, not sure what the difference is.

The functions push and pop generally only deal with the first or last items in a container.

[naraku9333]

This line students.push_back ( sect.students[ it ] ); doesn't make sense, you are using an iterator as an index to what I assume is a vector.
What is it exactly you are trying to accomplish?

[L1bXjE8b]

I know that I need push_back(). I have a sort function later on in the program in an output operator. I'm using a std::list. What I'm confused about is how to use push_back() with the iterator. Here is my whole .cpp file:

#include "section.h"
#include <list>
#include <iomanip>

using namespace std;

Section::Section ()
{
}

Section::Section (Course course, int callNumber)
{
}

Section::Section (const Section& sect)
{
  list<Student>::const_iterator it;
  for (it = sect.begin(); it != sect.end(); ++it)
    students.push_back ( sect.students[ it ] ); //problem here
}

Section& Section::operator= (const Section& sect) {
  if (this != &sect) {
  	   students.clear();
  }
    list<Student>::const_iterator it;
    for ( it = sect.begin(); it != sect.end (); ++ it ) {
        students.push_back ( sect.students[ it ] ); //fix this
    }

    for ( it = sect.begin(); it != sect.end(); ++it ) {
       addStudent (current->data); //not right
    }
  return *this;
}

Section::~Section() {
    students.clear();
}

void Section::addStudent(Student s) {
      students.push_back ( s );
    }

bool Section::operator== (const Section& right) const {
  return (theCourse == right.theCourse)
    && (theCallNumber == right.theCallNumber);
}

bool Section::operator< (const Section& right) const {
  if (theCourse < right.theCourse)
    return true;
  else if (theCourse == right.theCourse)
    return (theCallNumber < right.theCallNumber);
  else
    return false;
}

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

  out << section.getCourse() << section.getCallNumber() << " : ";
  out << section.getNumberOfStudents() << " students, ";

  Section* sections = new Section[fac.getNumberOfSections()];
  int totalEnrollment = 0;

  for ( list<Student>::const_iterator it = section.begin(); it < section.end() ++it)
    {
      sections[it] = section.sections[it];
      totalEnrollment += sections[it].getNumberOfStudents();
    }
  out << totalEnrollment << ", students\n";

  sort (sections, sections+section.getNumberOfSections());
  for ( list<Student>::const_iterator it = section.begin(); it != section.end(); ++it)
    {
      out << "  " << *it; //print the list 
    }

  out << endl;
  return out;

}

[TheIdeasMan]

What I'm confused about is how to use [ocde]push_back()[/code] with the iterator.

push_back adds an element to the end of the container, there is no need to use an iterator with it :+)

Instead of using the begin and end iterators to process the whole container, one can use a range based for loop:

for (const auto& element : section) {  // section is the name of the container variable
     out << "  " << element;  //print the list
}

http://en.cppreference.com/w/cpp/language/range-for

Hope this helps !!

Edit:

Constructors and other special member functions can be defaulted, in the header file:

Section () = default;
Section (const Section& sect) = default;
~Section() = default;

[L1bXjE8b]

Thank you. I have one question, though.

push_back adds an element to the end of the container, there is no need to use an iterator with it :+)

If I don't need an iterator or int i, then what will I put as the parameters of push_back()?

To add a student, I think this will work in my assignment operator:

  for ( it = sect.begin(); it != sect.end(); ++it ) {
     addStudent (*it);
  }

Is that legal?

[TheIdeasMan]

I am not sure, but you might not even need to default any of the special member functions(ctor's & dtor's): the implicit ones might be enough. One only needs to write one's own functions if they have something that needs special handling, such as owning raw pointers - but they should be rare. So you could possibly get rid of lines 7 to 39 :+)

If I don't need an iterator or int i, then what will I put as the parameters of push_back()?

See my example of the range based for loop: it would be push_back(element) You may want to rename element to something better: AStudent say.

To add a student, I think this will work in my assignment operator:

Yes, if you are to use iterators. Iterators are like pointers - you dereference them to get the actual value.

Do you have a leak with the new on line 64? One should avoid new and delete unless it is for low level stuff. Even then one can use smart pointers.

You shouldn't have the iterator as an array subscript. That would be the same as having a pointer as an array subscript. Anyway I don't think you need iterators at all.

Can we see your header files?

[L1bXjE8b]

Here is the header:

#ifndef SECTION_H
#define SECTION_H

#include <list>
#include <string>
#include <iostream>


#include "course.h"
#include "student.h"

class Section
{
  Course theCourse;
  int theCallNumber;

  std::list<Student> students;
  int numStudents;


public:
  typedef std::list<Student>::iterator iterator;
  typedef std::list<Student>::const_iterator const_iterator;


  Section ();
  Section (Course course, int callNumber);

  Section (const Section&);
  Section& operator= (const Section&);
  ~Section();

  const Course& getCourse() const {return theCourse;}
  int getCallNumber() const       {return theCallNumber;}

  int getNumberOfStudents() const {return numStudents;}
  void addStudent(Student s);

  bool operator== (const Section& right) const;
  bool operator< (const Section& right) const;

  iterator begin();
  const_iterator begin() const;

  iterator end();
  const_iterator end() const;


};


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

#endif 

Since the iterators are declared here, I think I need to use them.

[TheIdeasMan]

TheIdeasMan wrote:

So you could possibly get rid of lines 7 to 39 :+)

Sorry, that was a bit misleading in the respect that you still need to have an ordinary ctor :+)

With a ctor, make sure it initialises all the member variables. Use a member initialization list. Make the parameters names a little different to the member names - I like to have the member name with Arg appended.

http://en.cppreference.com/w/cpp/language/initializer_list

[TheIdeasMan]

Since the iterators are declared here, I think I need to use them.

Oh, right. So I am now guessing that the header file was given, and you need to do the cpp files and make it all work? Ha :+) that makes most of what I said irrelevant :+D Some of it still applies though.

With operator<< Why are you creating a new object and copying stuff into it? Why is there sorting? I don't think either of those things belong there. All the values should be known by this stage, all you are doing is telling the compiler how to print your object.

I see the header file already has declarations for the iterators, so no need to re-declare them when you need to use them.

[L1bXjE8b]

So I am now guessing that the header file was given, and you need to do the cpp files and make it all work?

Exactly.

Why is there sorting?

This prints a sorted list of elements, defined in another area of the program.

So when looking at the assignment operator, what is the best way to add a new student? Specifically, this:

list<Student>::const_iterator it;
  for ( it = sect.begin(); it != sect.end(); ++it ) {
      students.push_back ( it ); //Not right
  }

I know that I need to use the iterators, so I'd rather just used the const_iterator that is already declared. I understand how you used auto, but I don't understand how this works with an iterator--specifically, how this relates to the std::list.

[cire]

To get at the element referred to by an iterator, one must dereference it.

list<Student>::const_iterator it;
  for ( it = sect.begin(); it != sect.end(); ++it ) {
      students.push_back ( *it ); 
  }

[TheIdeasMan]

Why is there sorting?

This prints a sorted list of elements, defined in another area of the program.

But that is going to sort every time you call operator<<

I understand how you used auto, but I don't understand how this works with an iterator--specifically, how this relates to the std::list.

That was in relation to using the range based for loop, but you are using iterators instead, so I guess that doesn't apply. But so you know, range based for loop uses iterators implicitly (behind the scenes, auto-magically). The STL uses iterators a lot. The STL containers all define begin and end, and know how to get from one element to the next because all the elements are the same size. So one can use range based for loop on any of the STL containers.

I have to head out now - it's breakfast time at my end :+) It looks like cire is online - he is vastly more qualified to answer your questions :+) Cheers

[L1bXjE8b]

Thanks, TheIdeasMan. I appreciate your help.

Cire, I changed the areas in question to students.push_back ( *it );. When I do this, I do not get compilation errors, but this message is issued by the compiler:

Undefined symbols for architecture x86_64:
  "Section::end() const", referenced from:
      Section::Section(Section const&) in section.o
      Section::operator=(Section const&) in section.o
  "Section::begin() const", referenced from:
      Section::Section(Section const&) in section.o
      Section::operator=(Section const&) in section.o
  "operator<<(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, Section const&)", referenced from:
      operator<<(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, Faculty const&) in io_ops.o
      operator<<(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, Faculty const&) in section.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Any idea what this means or how to fix it?

[cire]

The linker could not find the definition for those functions. And indeed, I don't see their definitions (with the exception of operator<<) with the other definitions of Section functions in your code above. You need to supply them.
Perhaps something changed with regards to current code and operator<<?