Iterator constructor not initializing properly

Question

Im writing a HeapPriorityQueue class with an interator. The problem is when the .begin() fuction calls the Iterator constructer it goes into the copy assignment constructor for the base class(HeapPriorityQueue Class). I initialized all the variables but when they pass the the Iterator constructer they get lost. I dont understand why. I tried initializing them within the body but the debugger would not execute the body, which I thought was really weird.

I have just stripepd the code down enough so others can quickly understand. The only note I want to add is in order to determin the priority I am using a gt() fucntion, or tgt in the template.

Any help is appreciated, thanks

template<class T, bool (*tgt)(const T& a, const T& b) = nullptr> class HeapPriorityQueue {
  public:
    //Destructor/Constructors
    ~HeapPriorityQueue();

    HeapPriorityQueue          (bool (*cgt)(const T& a, const T& b) = nullptr);
    HeapPriorityQueue          (const HeapPriorityQueue<T,tgt>& to_copy, bool (*cgt)(const T& a, const T& b) = nullptr);

    class Iterator {
      public:
        //Private constructor called in begin/end, which are friends of HeapPriorityQueue<T,tgt>
        ~Iterator();
        T           erase();
        std::string str  () const;
        HeapPriorityQueue<T,tgt>::Iterator& operator ++ ();
        HeapPriorityQueue<T,tgt>::Iterator  operator ++ (int);


        friend Iterator HeapPriorityQueue<T,tgt>::begin () const;
        friend Iterator HeapPriorityQueue<T,tgt>::end   () const;

      private:
        //If can_erase is false, the value has been removed from "it" (++ does nothing)
        HeapPriorityQueue<T,tgt>  it;                 //copy of HPQ (from begin), to use as iterator via dequeue
        HeapPriorityQueue<T,tgt>* ref_pq= nullptr;
        int                      expected_mod_count=0;
        bool                     can_erase = true;

        //Called in friends begin/end
        //These constructors have different initializers (see it(...) in first one)
        Iterator(HeapPriorityQueue<T,tgt>* iterate_over, bool from_begin);    // Called by begin
        Iterator(HeapPriorityQueue<T,tgt>* iterate_over);                     // Called by end
    };


    Iterator begin () const;
    Iterator end   () const;


  private:
    bool (*gt) (const T& a, const T& b); // The gt used by enqueue (from template or constructor)
    T*  pq;                              // Array represents a heap, so it uses heap ordering property
    int length    = 0;                   //Physical length of pq array: must be >= .size()
    int used      = 0;                   //Amount of array used:  invariant: 0 <= used <= length
    int mod_count = 0;                   //For sensing concurrent modification
  };


template<class T, bool (*tgt)(const T& a, const T& b)>
HeapPriorityQueue<T,tgt>::HeapPriorityQueue(const HeapPriorityQueue<T,tgt>& to_copy, bool (*cgt)(const T& a, const T& b))
: gt(tgt != nullptr ? tgt : cgt), length(to_copy.length) {
    if (gt == nullptr)
        gt = to_copy.gt;//throw TemplateFunctionError("HeapPriorityQueue::copy constructor: neither specified");
      if (tgt != nullptr && cgt != nullptr && tgt != cgt)
        throw TemplateFunctionError("HeapPriorityQueue::copy constructor: both specified and different");

      pq = new T[length];

      if (gt == to_copy.gt)
        used = to_copy.used;

      for (int i=0; i<to_copy.used; ++i)
         enqueue(to_copy.pq[i]);
}

template<class T, bool (*tgt)(const T& a, const T& b)>
auto HeapPriorityQueue<T,tgt>::begin () const -> HeapPriorityQueue<T,tgt>::Iterator {
     return Iterator(const_cast<HeapPriorityQueue<T,tgt>*>(this),true);}

template<class T, bool (*tgt)(const T& a, const T& b)>
HeapPriorityQueue<T,tgt>::Iterator::Iterator(HeapPriorityQueue<T,tgt>* iterate_over, bool tgt_nullptr):
it(*ref_pq),ref_pq(iterate_over),expected_mod_count(ref_pq->mod_count){}

Think we need to see the `HeapPriorityQueue::begin` function, [but an MCVE would be preferred.](http://stackoverflow.com/help/mcve) — user4581301, Feb 17 '16 at 23:56
An iterator should _not_ have a copy of HPQ! Why would you do that? — Mooing Duck, Feb 17 '16 at 23:59

score 0 · Answer 1 · edited May 23 '17 at 11:59

I initialized all the variables but when they pass the the Iterator constructer they get lost.

Variables are always initialized in the order they're declared, so lets look at those:

    //If can_erase is false, the value has been removed from "it" (++ does nothing)
    HeapPriorityQueue<T,tgt>  it;                 //copy of HPQ (from begin), to use as iterator via dequeue
    HeapPriorityQueue<T,tgt>* ref_pq= nullptr;
    int                      expected_mod_count=0;
    bool                     can_erase = true;

and then:

HeapPriorityQueue<T,tgt>::Iterator::Iterator(HeapPriorityQueue<T,tgt>* iterate_over, bool tgt_nullptr):
it(*ref_pq),ref_pq(iterate_over),expected_mod_count(ref_pq->mod_count){}

First, you initialize it as *ref_pq, which is *nullptr, which is illegal and usually instantly crashes. I didn't really bother to analyze beyond that point, but I do have a question. Why in the world does your "pointer" contain a copy of the entire data set it's trying to iterate over? Don't do that. Also, I don't know what you intend with expected_mod_count, but that's probably also a bad idea. And so is your tgt template parameter.

For your review: Writing your own STL Container.

I know we're supposed to avoid comments like thanks, but thanks. That link is golden. — user4581301, Feb 18 '16 at 00:32

Iterator constructor not initializing properly

1 Answers1