2

I wrote the following C++ code to iterate over a vector of vectors of objects. I want to iterate over every object in the vector of vectors. The code below works but it has one peculiarity I don't understand.

The line "int types_size=types->size();" is a hack employed in iterator.hpp. I don't really know the language very well so I don't know if I've found a compiler bug or if this a bug in my code. The variable "types_size" shouldn't be needed. It is used in two lines

first line:

while(s<types_size && (s<0 || (*types)[s]->size()==0 || object==&( ((*types)[s])->end()))){

second line:

if(s<types_size){

If "types_size" is replaced with "types->size()" in the first line the code will seg fault when it runs. Making the same replacement but only in the second line does not result in a seg fault. I don't understand what is going on. Any comments on the rest of this code will be appreciated.

#ifndef _iterator_hpp_
#define _iterator_hpp_

#include <vector>
using namespace std;

typedef double Object;

typedef vector<Object> ObjectVector;

class Region{
public:
    vector<ObjectVector*> types;
};

class ObjectIterator{
private:
    int s;
    vector<ObjectVector*> *types;
protected:

public:
    Object *object;
    bool finished;

    ObjectIterator operator++(){
        if (s>=0) object++; // increments to next object
        int types_size=types->size();    // this is a hack that fixes a seg fault (compiler bug??)
        // *types is a vector<ObjectVector*>. (*types)[s] is the "sth" ObjectVector*.
        // &(*iterator) gives a pointer to the object pointed to by iterator.
        //((*types)[s])->end()) is an iterator that points past the end of
        // the ObjectVector* ((*types)[s])->end())
        while(s<types_size && (s<0 || (*types)[s]->size()==0 || object==&(*      ((*types)[s])->end()))){
            //need to increment to next non-empty types
            s++;
            if(s<types_size){
                object=&((*(*types)[s])[0]);
            }else{finished=true;}
        }
        return (*this);
    }

    /*---------------------constructor--------------------------------------------------------

      start with s=-1 and increment to first object */

    ObjectIterator(vector<ObjectVector*> *typesarg):finished(false) {
        types=typesarg;s=-1;++(*this);
    };

};

#endif

--------------------------------main--------------------------------

// it.cpp

//  g++ it.pp 

#include <iostream>

#include <vector>

#include "iterator.hpp"

using namespace std;

int num_types=3;

int main(){

  Region region;

  int num_objects[num_types];

  num_objects[0]=1;
  num_objects[1]=3;
  num_objects[2]=5;

  // create an ObjectList for each type 


  for(int s=0;s<num_types;s++){
    ObjectVector *objectlist = new ObjectVector;

    for(int i=0;i<num_objects[s];i++){
      objectlist->push_back((double)2*(i+1)*(s+1));
    }
    region.types.push_back(objectlist);
  }

  cout <<"types.size="<< region.types.size()<<endl;

  for(ObjectIterator OI(&region.types); !OI.finished ; ++OI)
    {
      cout <<*(OI.object)<<endl;
    }


}
user1688949
  • 131
  • 7
  • Read your question a bit briefly, but I think an obvious answer would be that types->size() is returning something not of type int, and your line "int types_size=types->size()" is taking the other type and casting it as an int. For example, if you have double a = 3.2; and int b = a; int b will be equal to 3. – Shawn Dec 16 '16 at 17:17
  • 2
    Can you produce a [MCVE] – Lightness Races in Orbit Dec 16 '16 at 17:21
  • @LightnessRacesinOrbit although very ugly, this compiles. – vincent Dec 16 '16 at 17:24
  • "compiler bug or bug in my code". 99% of the time it will be the latter, especially if it fails on more than one compiler. – Barmar Dec 16 '16 at 17:24
  • 1
    @ShawnS `vector::size()` returns `size_t`, which is an integer type. – Barmar Dec 16 '16 at 17:27
  • 1
    @vincent: I didn't say it wouldn't. – Lightness Races in Orbit Dec 16 '16 at 17:29
  • The code runs correctly with types->size() cast to an int. In light of Barmar's comment I don't know what to make of this. (The comment that vector::size() returns size_t, which is an integer type.). – user1688949 Dec 16 '16 at 17:34
  • The code runs. To see it break make the replacement: s ssize() – user1688949 Dec 16 '16 at 17:37
  • `types.size()` or `types->size()`? – Barmar Dec 16 '16 at 17:37
  • Barmar my last comment had the typo. I meant types=>size() – user1688949 Dec 16 '16 at 17:38
  • Why does it always set `object` to the `[0]` element of the vector? – Barmar Dec 16 '16 at 17:39
  • 1
    It seems like you're making this more complex than it needs to be. The iterator should have 2 index variables, `i1` is the index into the top-level vector, `i2` is the index into the current sub-vector. You increment `i2`, and when `i2 >= types[i1].size()` you increment `i1` and set `i2` to `0`. – Barmar Dec 16 '16 at 17:43
  • 1
    The proper way to figure this out is to use a debugger. When you get the segfault, check the values of all the variables and see what's wrong. – Barmar Dec 16 '16 at 18:04
  • Why does it work if you get rid of the dereference and change "cout <<*(OI.object)< – vincent Dec 16 '16 at 18:41
  • It goes into an infinite loop if I remove the dereference operator. It also goes into an infinite loop if I remove the line cout <<*(OI.object)< – user1688949 Dec 16 '16 at 19:06

1 Answers1

1

size() returns an unsigned integer so this:

while(s<types->size()

will not behave the same if s is -1 (its initial value). -1 is turned into a large unsigned integer and the comparison is false. See Signed/unsigned comparisons for more information.

Community
  • 1
  • 1
Olivier
  • 1,144
  • 1
  • 8
  • 15