Jump to content
  • Advertisement
Sign in to follow this  
cignox1

iterators as class members

This topic is 5003 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi, I've just begun to use STL and I have a question: I use an iterator for a list to iterate the list in a loop and the method that does this is called millions of times. Since now I create the two iterators each time the function is called (there are two lists) I thought that putting the iterator as class member and only initialize it when I need it could speed up a bit the algorithm (I don't expect to double the speed, just a small improvement). Currently I get an error if I move the iterator in the class declaration. What should I do?

Share this post


Link to post
Share on other sites
Advertisement
You will need to add typename as a prefix to it. I did not know that either until someone told me. Either that or typedef but I think the latter. Here's what I have:

typedef std::hash_map < const std::string, tBuffer >::iterator Iterator;
...
Iterator my_itr;


Although I swear I had to use typename somewhere...

Share this post


Link to post
Share on other sites
yea, as Drew mention, thats a common problem with templates its called 'dependant typing'

template< class T >
void myFunction() {
T::N *a; // the compiler assumes a name qualified by a template parameter
// refers to a static member (it has to assume something, since T
// is unknown) so it basicaly thinks this is a multiplication op

typename T::N *a; // typename tells the compiler, that T::N is a type
}

Share this post


Link to post
Share on other sites
Quote:
Original post by Drew_Benton
You will need to add typename as a prefix to it. I did not know that either until someone told me. Either that or typedef but I think the latter. Here's what I have:

typedef std::hash_map < const std::string, tBuffer >::iterator Iterator;
...
Iterator my_itr;


Although I swear I had to use typename somewhere...

You only have to use typename if the hash_map instantiation is dependent on a template parameter.

Share this post


Link to post
Share on other sites
That is what I have, following your idea:

typedef list<QLLight*>::iterator l_iter;
list<QLLight*> lights;

and then, in the class declaration:

l_iter l;

In the method I then use:

l = lights.begin();

But the compiler gives me the following error:
41 C:\Alessandro\progetti\quarklight\scene.cpp no match for 'operator=' in '((const QLScene*)this)->QLScene::l = (((const std::list<QLLight*, std::allocator<QLLight*> >*)((const QLScene*)this)) + 32u)->std::list<_Tp, _Alloc>::begin [with _Tp = QLLight*, _Alloc = std::allocator<QLLight*>]()'

[edit]I explained my problem in the wrong way: I'm not writing a template, I'm writing a class using a STL template (in this case list) but the compiler doesn't like a class member list.

Share this post


Link to post
Share on other sites
Quote:
Original post by cignox1
But the compiler gives me the following error:
41 C:\Alessandro\progetti\quarklight\scene.cpp no match for 'operator=' in '((const QLScene*)this)->QLScene::l = (((const std::list<QLLight*, std::allocator<QLLight*> >*)((const QLScene*)this)) + 32u)->std::list<_Tp, _Alloc>::begin [with _Tp = QLLight*, _Alloc = std::allocator<QLLight*>]()'


I've highlighted the problem, you must be using that piece of code inside a constant member function in which case you need to use a constant iterator instead i.e.:


struct foo {

typedef list<QLLight*>::iterator l_iter;
typedef list<QLLight*>::const_iterator const_l_iter;

list<QLLight*> lights;

.....
void bar() const {
const_l_iter l = lights.begin();
......
}
};


Thats the great thing about constant correct-ness the compiler will stop you from doing something silly [smile]

Share this post


Link to post
Share on other sites
Thank you for pointing out this. I made the change, but now another error occours:
41 C:\Alessandro\progetti\quarklight\scene.cpp passing `const l_iter' as `this' argument of `std::_List_const_iterator<QLLight*>& std::_List_const_iterator<QLLight*>::operator=(const std::_List_const_iterator<QLLight*>&)' discards qualifiers

It compiles if I un-const the method. I will do this way for every method that will use the iterator.
Thank you all!

Share this post


Link to post
Share on other sites
Quote:
Original post by cignox1
Thank you for pointing out this. I made the change, but now another error occours:
41 C:\Alessandro\progetti\quarklight\scene.cpp passing `const l_iter' as `this' argument of `std::_List_const_iterator<QLLight*>& std::_List_const_iterator<QLLight*>::operator=(const std::_List_const_iterator<QLLight*>&)' discards qualifiers


Okay i think you need to should show us some code. You can't just put const on a non-constant iterator. a constant iterator is a real type well thats what it looks like your doing.

Quote:
Original post by cignox1
It compiles if I un-const the method. I will do this way for every method that will use the iterator.


That is not a good idea, when you do operations on container that does not modify the state of the container at all you should use constant iterators.

Share this post


Link to post
Share on other sites

typedef list<QLLight*>::const_iterator l_iter;
//Definition of the classes
class QLScene
{
public:
string name;

vector<QLCamera*> cameras;
list<QLLight*> lights;
l_iter l;
.
.
.
};


This is how I declared my class, lists and iterators, and the following method, if const, produces the already posted error:

QLLight* QLScene::GetLight(uint index) const
{
if(index > lights.size()) return null;

l = lights.begin(); //Here an error
for(int i = 0; i < index; i++) ++l; //Here a similar one

return *l;
}


If I remove const from the method, it compiles.

Share this post


Link to post
Share on other sites
Okay i can see the problem [smile], your trying to modify the state of a data member inside a constant member function which you can't do unless it's a mutable data member. Either make the data member mutable (it's not generally a good idea but it depends on context) or just create a new local variable so either:

A:


#include <cstddef> // size_t
#include <stdexcept> // out_of_range
#include <iterator> // advance
#include <list>
#include <vector>
#include <string>

namespace QL {

struct Camera;
struct Light;

struct Scene {

typedef std::vector<Camera*> camera_list;
typedef std::list<Light*> light_list;
typedef light_list::iterator light_itr;
typedef light_list::const_iterator const_light_itr;
typedef std::size_t size_type;

private:

std::string name;
std::vector<Camera*> cameras;
std::list<Light*> lights;
light_itr curr_itr;

public:

const Light& GetLight(size_type index) const {

if(index > lights.size())
throw std::out_of_range("index out of range");

const_light_itr l = lights.begin();
std::advance(l, index);
return **l;
}
};
};


or B:


#include <cstddef> // size_t
#include <iterator> // advance
#include <list>
#include <vector>
#include <string>

namespace QL {

struct Camera;
struct Light;

struct Scene {

typedef std::vector<Camera*> camera_list;
typedef std::list<Light*> light_list;
typedef light_list::iterator light_itr;
typedef light_list::const_iterator const_light_itr;
typedef std::size_t size_type;

private:

std::string name;
std::vector<Camera*> cameras;
std::list<Light*> lights;
mutable const_light_itr curr_itr;

public:

const Light& GetLight(size_type index) const {

if(index > lights.size())
throw std::out_of_range("index out of range");

curr_itr = lights.begin();
std::advance(curr_itr, index);
return **curr_itr;
}
};
};


notice how namespaces make prefixing type-names redundant and more cleaner [wink]

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!