Click here to Skip to main content
15,892,005 members
Please Sign up or sign in to vote.
3.00/5 (3 votes)
hi every one !
i have probleme with the resuse of the same instance, here is my code :
C++
vector<ConnectedComponent*> cc_tab1 ;
		while(!queue.empty())
		{
			
			marked=Breadth_First_Search(queue[0]);
		   ConnectedComponent	*cc;
		   cc =new ConnectedComponent;
			for(int i=0;i<marked.size();i++)

			{		
				cout <<"manelll"<< endl; 
				if(marked[i]==true)
				{
					string f= m_vertices[i];
					cout<<"m_nodes.push_back(f)" << endl;
					cc->m_nodes.push_back(f);
					cout<<"cc.m_nodes["<<i<<"]="<< cc->m_nodes[i]<< endl;
					queue.erase( queue.begin());
				}
			}
			cc_tab1.push_back(cc);
			cout << "manouuuuuuuuuuuuuuuuuuuu"<< endl;
			cc->m_nodes.clear();
			delete cc;
			k++;
			cout << "k=" << k<< endl;
		}

here i'm trying to search how much ConnectedComponenet do i have, so i create an instance (*cc)of ConnectedComponent, and i use it to full out my vector cc_tab1, after that i Register it , and i clear the varibles of my instance after i delete it, that works just for one iteration and it shows me this error :
C++
Expression : vector subscript out of range


could any one help me please

Thank you verry much
Posted
Comments
Ian A Davidson 14-Aug-13 19:56pm    
Can you say which line the error occurs on?
As OriginalGriff points out, you are deleting an object after you've placed it in your vector, but I don't think that's causing the problem you are seeing (once you've fixed that and if you don't correct the other mistake, I expect you'll memory access violation error later in the code, when you try to use cc_tab1).
I suspect the error you are seeing is in the for loop, right?
Question: What does Breadth_First_Search do? You pass it the first item in "queue", and then iterate through. But then in the for loop you're removing the first item in the queue.
What about m_vertices? Can you be sure there are the same number of items in it as there are in "marked"?
Finally, you are also referencing cc->m_nodes by index (i). But you are only adding a node if "marked[i]" is true. So there may not be the same number of nodes. Do you want to use something like "m_nodes->back()" instead?
Quite frankly, the code seems extremely hard to understand. I think it needs some serious thought about what you are trying to do. Use comments as you go along, not only to explain it to someone else, but also to force yourself to think about what you are doing and check it would work or whether you need other checks (for whether you indexes are in range, for example).
Regards,
Ian.
Manel1989 15-Aug-13 3:16am    
So sorry i didn't write comments, you are right : i will explain : i have an xml file which have all my nodes and my connected component, i have to find out how much connected component do i have and in every one what are the nodes in it ok , that's whay i use Breadth_First_Search to explore my graph from a node that i give to it , once it's done it will give me in return a bool vector telling if each node is connected to the node (root ) that i gave , if its true rturns true else returns false, m_vertices is a vector that has all my nodes so it has the same number of items in it as "marked ", what i'm trying to do is : i create a vector of COnnectedComponent (cc_tab1) in it i would put all my connected component in my origial graph, therefor , i create an instance of ConnectedComponent cc that i think i can use it to fill out the cc_tab1 , firstable , i iterate through , test if marked[i] is true ( if it is true that means that this node belongs to this connected component ), i extarct the node from m_vertices then i push it back to the vector m_nodes of this connected component, after that i erase this node from queue( so i just have left the nodes that are not in my connected component cc) finely i add this connected componet to cc_tab1 and look for another connected component using the same instance cc.
if , i dont use cc->m_nodes.clear();
delete cc;
i get in cc_tab1[0]->m_nodes (N0;N1,N2,N3) but in cc_tab1[1]->m_nodes (N0;N1,N2,N3,N4,N5) and that's wrong it is suposed to have just (N4 ,N5) and that is my problem , could you please help me i really tried every thing i konw , thank you verry much for your help

This is called object pooling in general. Its an optimization that can speed up an algorithm and make its memory use much more efficient if it involves a lot of object creation/deletion. First thing to consider: do you need this optimization??? Don't optimize something that runs with acceptable speed. Optimization can often introduce evil bugs because it makes your code more complex.
If you still insist on pooling your objects: You have to hide the implementation of object creation/deletion of the objects from the code that actually uses the object. The pool is the object that hides this detail.

C++
// Provided as is without warranty. Untested.
template <typename T>
class ObjectPool
{
public:
    ObjectPool(size_t max_size=100)
        : m_MaxSize(max_size)
    {}
    ~ObjectPool()
    {
        // We do not optimize here, keeping the code readable is more important
        // as we are creating and deleting an object pool rarely...
        SetMaxPoolSize(0);
    }
    void SetMaxPoolSize(size_t size)
    {
        m_MaxSize = size;
        size_t current_size = m_Objects.size();
        if (current_size > size)
        {
            for (size_t i=size; i<current_size; ++i)
                delete m_Objects[i];
            m_Objects.resize(size);
        }
    }
    T* AcquireObject()
    {
        if (m_Objects.empty())
            return new T;
        T* obj = m_Objects.back();
        m_Objects.pop_back();
        return obj;
    }
    void ReleaseObject(T* obj)
    {
        if (m_Objects.size() < m_MaxSize)
            m_Objects.push_back(obj);
        else
            delete obj;
    }
private:
    std::vector<T*> m_Objects;
    size_t m_MaxSize;
};

struct S
{
    void ObjMethod()
    {
        printf("%s\n", __FUNCTION__);
    }
};

void PoolTest()
{
    ObjectPool<S> op;
    S* obj = op.AcquireObject();

    // TODO: use the object
    obj->ObjMethod();

    op.ReleaseObject(obj);
}


// Provided as is without warranty. Untested.
template <typename T>
class SmartPointerLikeSomethingThatAutomaticallyReleaseTheObject
{
public:
    SmartPointerLikeSomethingThatAutomaticallyReleaseTheObject(ObjectPool<T>& pool)
        : m_Pool(pool)
    {
        m_Ptr = pool.AcquireObject();
    }
    ~SmartPointerLikeSomethingThatAutomaticallyReleaseTheObject()
    {
        Release();
    }
    void Release()
    {
        if (m_Ptr)
        {
            m_Pool.ReleaseObject(m_Ptr);
            m_Ptr = NULL;
        }
    }
    T* operator->()
    {
        assert(m_Ptr);
        return m_Ptr;
    }
    T& operator*()
    {
        assert(m_Ptr);
        return *m_Ptr;
    }
private:
    // disabling copy constructor
    SmartPointerLikeSomethingThatAutomaticallyReleaseTheObject(const SmartPointerLikeSomethingThatAutomaticallyReleaseTheObject&);
private:
    ObjectPool<T>& m_Pool;
    T* m_Ptr;
};

void PoolTestWithRAII()
{
    ObjectPool<S> op;

    SmartPointerLikeSomethingThatAutomaticallyReleaseTheObject<S> obj(op);
    // use obj, it gets release automatically when it goes out of scope (at the end of function)
    obj->ObjMethod();

    {
        SmartPointerLikeSomethingThatAutomaticallyReleaseTheObject<S> obj2(op);
        // use obj, it gets release automatically when it goes out of scope (at the end of function)
        obj2->ObjMethod();
    }

    printf("End of function\n");
}


EDIT: You can also implement a "null pool" that never pools anything, creates the object when you acquire it and it always deletes when you release the object. If you want to switch between a null pool and a normal pool at compile time you can do that with a define, or you can implement a common interface in a null pool and a normal pool if you want to switch between them at runtime.
 
Share this answer
 
v3
Comments
Sergey Alexandrovich Kryukov 14-Aug-13 12:23pm    
Well, a lot of good stuff here, but I think this is a total overkill, as far as this question is concerned. My 5 anyway.
—SA
pasztorpisti 14-Aug-13 14:33pm    
Thank you! This can be put together in 5 minutes and I had the time. Overkill???
Sergey Alexandrovich Kryukov 14-Aug-13 15:08pm    
Not from this point, but from the point of view of OP as I can picture it. I think the question was way more trivial, something about having a pointer to some object and passing it to some other object and/or method...
—SA
pasztorpisti 14-Aug-13 15:22pm    
You must be right. I had similar thought when I saw the discussion about "delete". But since the topic was about object pooling (a quite advanced topic) I thought that seeing a good example on how to do this wouldn't hurt...
Sergey Alexandrovich Kryukov 14-Aug-13 15:32pm    
Oh, it would not hurt, not at all. It was just a note...
—SA
You are preserving the pointer to an object in your cc_tab1, and then deleting the object you have just pushed back.

Anything which tries to use deleted objects will give you a problem: why are you deleting an object if you haven't finished with it?
 
Share this answer
 
Comments
Manel1989 14-Aug-13 12:17pm    
i have to reuse the object cc to fill out the cc_tab1 , honnestly i don't know how to reuse an instance, but , if there is another way to find out how much connectedComponent do i have in my graph can you help me , thank you
hi !
since i'm new in c++ , i just tryed to find a simple way for me to get what i want , i know it is not the best solution but i worked for me , i wish you give your opinion about it thank you :
C++
vector<connectedcomponent> ConnectedComponentSearch::CC_calculation()
{

	vector<bool> marked;
	vector<connectedcomponent> cc_tab ;
	ConnectedComponent cc;
	vector<string> queue ;
	vector<string> cc_nodes ;//a vector that will have the node of a Connected Component , we will use it in the comparaison

	for(int i=0;i<m_vertices_number;i++)>
		{
			queue.push_back(m_vertices[i]);
			cout<<"queue["<<i<< "] =" <<queue[i]<<endl;
		}
	cout << "queue[0]=" << queue[0];
	string f= queue[0];
		marked=Breadth_First_Search(f);
		for(int i=0;i<marked.size();i++)>
		{
			cout<<"marked["<< i<< "] ="<< marked[i]<<endl;
		}

		while(!queue.empty())
		{
			
			marked=Breadth_First_Search(queue[0]);
			for(int i=0;i<marked.size();i++)>

			{	
				if (marked[i]==true)
				{
					string f= m_vertices[i];
					cout<<"m_nodes.push_back(f)" << endl;
					cc.m_nodes.push_back(f);
					cout<<"cc.m_nodes["<<i<<"]="<< cc.m_nodes[i]<< endl;
					queue.erase( queue.begin());
				}
				
			    
			}
			cc_tab.push_back(cc);
			k++;

		}


		cout<<"******The connected components in the graph*********"<<endl;
		for (int i=0; i<k;i++)>
		{
			for(vector<connectedcomponent>::size_type j = 0; j != cc_tab[i].m_nodes.size(); ++j)
            {
   
			cout << cc_tab[i].m_nodes[j] << endl;
	        }
		}
		int m=0;
		
		for (int l=0;l<k-1;l++)>
		{
			for (int z=m; z<k-1;z++)>
			{
			for(vector<connectedcomponent>::size_type j = 0; j != cc_tab[m].m_nodes.size(); ++j)
			{ 
			    string f= cc_tab[m].m_nodes[j];
				if(search(cc_tab[z+1].m_nodes,f)==true)
				{
					cc_tab[z+1].m_nodes.erase(cc_tab[z+1].m_nodes.begin());
			    }
			}
			}
			m++;
		}

		for (int l=0;l<k;l++)>
		{
			for(vector<connectedcomponent>::size_type j = 0; j != cc_tab[l].m_nodes.size(); ++j)
			{  
				cout << cc_tab[l].m_nodes[j] << endl;
			}
		}

		for(int i=0;i<maxp.size();i++)>
			cout<<maxp.at(i)<<"__"<<endl;
		return cc_tab;

}</connectedcomponent></connectedcomponent></connectedcomponent></string></string></connectedcomponent></bool></connectedcomponent>

thank you all for your help :) good night
 
Share this answer
 
Comments
pasztorpisti 16-Aug-13 3:01am    
This is called spaghetti code. :-)
Manel1989 16-Aug-13 6:59am    
spaghetti ?? whayyyy ???????????

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900