Using push_back() with vector causing segmentation fault

[L1bXjE8b]

I am trying to use push_back() with a vector of type Section--i.e., vector<Section> sections. I am trying to use push_back() to add a copy from the copy constructor into the copied version. Here is my constructor:

Faculty::Faculty (const Faculty& f)
: theName( f.theName )
{
  for (int i = 0; i < f.sections.size(); ++i) {
    f.sections.push_back ( sections.theName ); 
  }
}

What am I doing wrong here? I am getting the error: "No matching member function call to 'push_back'. I don't understand why push_back() is giving me trouble, because I was able to successfully use other vector-specific functions such as size().

sections.size() works, but sections.push_back() and sections.reserve() do not.

[naraku9333]

You are trying to modify a constant object. Perhaps you mean to call push_back on this->sections?

[L1bXjE8b]

Are you saying try
sections.push_back ( f.theName ); ?

I tried it, but that doesn't work. Basically, all this is trying to do is say, "if there is room, add a new section into the copy--i.e., f. I'm not sure if I should be doing f.sections.push_back or just sections.push_back. Any ideas?

[helios]

this->sections can't possibly have a member named theName, since it's an std::vector. It looks like you meant to write this:

for (int i = 0; i < f.sections.size(); ++i) {
    f.sections.push_back ( sections[i] ); 
}

But since the new copy is this, not f, what you really meant to write was this:

for (int i = 0; i < f.sections.size(); ++i) {
    sections.push_back ( f.sections[i] ); 
}

Which itself is equivalent to this:

sections = f.sections;

Even better:

Faculty::Faculty (const Faculty& f): theName( f.theName ), sections( f.sections ){}

Note that if Faculty has no other members, this copy constructor is equivalent to the one generated by default by the compiler if you don't define it explicitly.

class Faculty{
    std::string theName;
    std::vector<?> sections;
public:
    //...
    // Redundant:
    // Faculty::Faculty (const Faculty& f): theName( f.theName ), sections( f.sections ){}
    //...
};

[L1bXjE8b]

Thanks for the suggestion, Helios. sections.push_back ( f.sections[i] ) fixed the compilation error. Now, however, I'm getting a segmentation fault, and I'm not sure where my code is running into a problem. Any ideas? Here is my faculty.h:

#ifndef FACULTY_H
#define FACULTY_H

#include <string>
#include <iostream>
#include <vector>
#include <algorithm>


#include "name.h"
#include "section.h"
using namespace std;

class Faculty
{
  Name theName;
  public:
  Faculty (int maxSections = 100);
  Faculty (Name name, int maxSections = 100);

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

  Name getName() const {return theName;}

  //No longer need numSections -- replaced by size_type size()
  int getNumberOfSections() const {return sections.size( );}

  const Section& getSection (int callNumber) const; 

  Section& getSection (int callNumber); 

  bool containsSection (int callNumber) const; 
  void addSection(Section s);

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

private:
  //This should be correct...
  std::vector <Section> sections; //vector of type Section
  friend std::ostream& operator<< (std::ostream& out, const Faculty& fac);
};

std::ostream& operator<< (std::ostream& out, const Faculty& fac);

#endif 

And here is faculty.cpp:

#include "faculty.h"

#include <cassert>
#include <vector>

using namespace std;

Faculty::Faculty (int maxSections)
{
}

Faculty::Faculty (Name name, int maxSections)
  : theName(name)
{
}

Faculty::Faculty (const Faculty& f)
: theName( f.theName ) {
  for (int i = 0; i < f.sections.size(); ++i) {
    sections.push_back ( f.sections[i] ); 
  }
}

Faculty& Faculty::operator= ( const Faculty& f )
{
  if ( this != &f )
    {
      theName = f.theName;
      for ( int i = 0; i < f.sections.max_size(); ++i )
        sections.push_back ( f.sections[i] );
    }
  return *this;
}

Faculty::~Faculty()
{
}

const Section& Faculty::getSection ( int callNumber ) const
{
  int i = 0;
  while ( i < sections.size() && sections[i].getCallNumber() != callNumber )
    ++i;

  assert ( i < sections.size() );
  return sections[i]; 
}

Section& Faculty::getSection ( int callNumber )
{
  int i = 0;
  while ( i < sections.size() && sections[i].getCallNumber() != callNumber )
    ++i;

  assert ( i < sections.size() );
  return sections[i];
}

bool Faculty::containsSection ( int callNumber ) const
{
  int i = 0;
  while ( i < sections.size() && sections[i].getCallNumber() != callNumber )
    ++i;

  return ( i < sections.size() );
}

void Faculty::addSection( Section s )
{
  assert ( sections.size() < sections.max_size() );
  sections[ sections.size() ]= s;
  for ( vector<Section>::size_type i = 0; i < sections.size(); ++i ) {
      sections.push_back ( s );
  }
  if ( sections.size() == sections.max_size () ) {
    sections.reserve ( 2 * sections.max_size() + 1 );
  }
}

bool Faculty::operator== ( const Faculty& right ) const
{
  return getName() == right.getName();
}

bool Faculty::operator< ( const Faculty& right ) const
{
  return getName() < right.getName();
}

What am I missing here? It seems like something so simple, but I can't figure it out.

[helios]

std::vector::max_size() returns the maximum size the vector could ever possibly reach due to system limitations, not the capacity. std::vector::capacity() returns the maximum number of elements the vector can hold in its current internal array, before performing a reallocation.

//By definition, this condition is always true, no matter what.
assert ( sections.size() < sections.max_size() );

//...

//Remove this code. std::vector does this automatically:
if ( sections.size() == sections.max_size () ) {
    sections.reserve ( 2 * sections.max_size() + 1 );
}

[L1bXjE8b]

I realized that between the original comment and when you responded, so I took it out. However, I still have a segmentation fault 11. Perhaps it has to do with the

sections[sections.size()] = s;
for ( vector<Section>::size_type i = 0; i < sections.size(); ++i ) {
    sections.push_back ( s );

Is it correct to be using push_back() on s?

The new addSection function:

void Faculty::addSection( Section s )
{
  //assert ( sections.size() < sections.max_size() );
  sections[ sections.size() ]= s;
  //for each section, push_back the new section
  for ( vector<Section>::size_type i = 0; i < sections.size(); ++i ) {
  sections.push_back ( s );
  }

  //If the sections are full, reserve twice the max_size.
  //Copy the new, reserved size into size()
  //if ( sections.size() == sections.max_size () ) {
  //sections.reserve ( 2 * sections.max_size() + 1 );
  //}
}

[helios]

Yes, I missed that. If x is an std::vector, x[x.size()] is never valid code.

Also, this loop:

for ( vector<Section>::size_type i = 0; i < sections.size(); ++i ) {
  sections.push_back ( s );
}

If sections.size() > 0 at the start of the loop, the loop will continue until memory allocation fails.

A single push_back() should do what you want.

[L1bXjE8b]

So, you're saying change the addSection function to:

void Faculty::addSection( Section s ) {
  assert ( sections.size() < sections.max_size() );
  sections.push_back ( s );
}

?

Unfortunately, the segmentation fault is still present. Could this loop, in both the copy constructor and the assignment operator be a problem for the same reason?

for ( int i = 0; i < f.sections.max_size(); ++i )
        sections.push_back ( f.sections[i] );

[helios]

That Faculty::addSection() is correct, but remove that assert(). It doesn't do anything.

Stop calling std::vector::max_size(). It does nothing you need. std::vector::size() is what you want.

[L1bXjE8b]

Removing max_size() fixed my problem! Thank you so much, Helios!