Sign in to follow this  

STL vector...

This topic is 4763 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

Jeez, I hate having two threads on the same forum but I have so many problems that I have no choice!? I have a class which contains an STL vector of pointers to class ScreenEntityBase. In one of the methods within this class I need to be able to get a pointer to a specific ScreenEntityBase object within the list so that I can invoke methods within it. So I created a function called FindEntity (see class).
class Screen
{
#ifndef SCREEN_H
#define SCREEN_H

#include "ScreenEntityAnimation.h"
#include "ScreenEntityStaticImage.h"
#include "ScreenEntityText.h"

// Screen class
class Screen
{
public:
        ScreenEntityBase* FindEntity(WCHAR* SearchString);
	
private:
	vector<ScreenEntityBase*> ScreenEntityList;
};

#endif
}

How should I define this method? I had thought this might work:
ScreenEntityBase* Screen::FindEntity(WCHAR* SearchString)
{
        for(vector<ScreenEntityBase*>::iterator walker = ScreenEntityList.begin();walker != ScreenEntityList.end();walker++)
        {
                if(wcscmp((walker*)->GetName(), SearchString)
                {
                        return (walker*);
                        break;
                }
        }
        return 0;
}

Will this work? Thanks in advance. Mark Coleman

Share this post


Link to post
Share on other sites
I think you want *walker instead of walker*

I don't know if it will make a difference on your compiler, but you might want to do "++walker" instead of "walker++" in your for loop. Using the post-increment your compiler might make a copy of the walker object before it does the actual increment.

Share this post


Link to post
Share on other sites
What are the access patterns for ScreenEntityList like? If most of the time you're prosessing the entire list then vector is probably a good choice (although deque may be better depending on its growth patterns). If you often need to access elements by name then a map may be a better way to go.

Also, prefer algorithms to hand-written loops:
template <class TYPE>
class GetByName
{
public:
GetByName(std::wstring name)
:
name_(name)
{
}
bool operator()(const TYPE& object) const
{
return object->GetName() == name_;
}
private:
std::wstring name_;
};

ScreenEntityBase* Screen::FindEntity(std::wstring SearchString)
{
ScreenEntityBase* entity = std::find_if(ScreenEntityList.begin(), ScreenEntityList.end(), GetByName<std::wstring>(SearchString));
if (entity == ScreenEntityList.end())
{
return null;
}
return *entity;
}


Enigma

Share this post


Link to post
Share on other sites
You don't need the break statement after the return, since you're returning execution can't continue anyway :)

Also, if you're calling FindEntity() a lot and there are a fair few entries in your vector then it might be worth checking out std::map. Using std::map it would look something like this.....

class Screen
{
public:
ScreenEntityBase* FindEntity(const std::wstring& SearchString);
private:
typedef std::map<std::wstring,ScreenEntityBase*> ScreenEntityMap;
ScreenEntityMap screenEntities;
};

ScreenEntityBase* Screen::FindEntity(const std::wstring& SearchString)
{
ScreenEntityMap::iterator it = screenEntities.find(SearchString);
return (it!=screenEntities.end() ? *it : NULL);
}

Share this post


Link to post
Share on other sites
Quote:
Original post by Enigma
Also, prefer algorithms to hand-written loops:

For anything more substantial I'd agree, but IMHO with simple cases such as this your only making it harder to read. Switching to std::wstring is good thing though.

Share this post


Link to post
Share on other sites
Wow! So much input guys!

Most of the access is sequential and there are only ever 5-50 elements in the list at any one time so speed isn't really an issue.

Also I will drop the break!

A big thank you.

Mark Coleman

Share this post


Link to post
Share on other sites

This topic is 4763 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.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this