CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Page 1 of 4 1234 LastLast
Results 1 to 15 of 48
  1. #1
    Join Date
    May 2009
    Posts
    22

    Function only returns part of the object? :S

    Hello everyone. I have a problem thats really confusing me. It's probably something easy, but I don't have any experience in c++ or c.

    I'm calling a function to find the best Node in a vector of leaf nodes and to return it. Everything seems to be returned fine, except for the variable "parent", that is part of the Node struct and points to the parent node.
    Here is the important part of the code i think (let me know if i need to post more code):

    Code:
    current = getBest();
    std::cout<<"best parent sample: "<<(*current.parent).sampleValue<<std::endl;
    std::cout<<"best parent memory: "<<(current.parent)<<std::endl;
    And here are the final 2 lines of getBest():

    Code:
    std::cout<<"inside getBest: best parent sample: "<<(*(leafs[bestIndex].parent)).sampleValue<<std::endl;
    std::cout<<"inside getBest: best parent memory: "<<(leafs[bestIndex].parent)<<std::endl;
    return leafs[bestIndex];
    This is the output:

    inside getBest: best parent sample: 2
    inside getBest: best parent memory: 0xbffff6a0
    best parent sample: 65
    best parent memory: 0xbffff6a0

    So before the return statement it has the right value, but after the return is assigned to "current" it suddenly isn't right anymore. But the memory address is the same. I don't get it. Is this something familiar?

    Thanks

  2. #2
    Join Date
    Oct 2008
    Location
    Singapore
    Posts
    195

    Re: Function only returns part of the object? :S

    My guess is that you are allocating an entire linked list array in getBest. So when it returns, all notes are deallocated and the node pointed to by the 'best' node points to a deallocated space.
    But if getBest is accessing a global linked list, then it's quite surprising. It would help if you could post the entire getBest function.

  3. #3
    Join Date
    May 2009
    Posts
    22

    Re: Function only returns part of the object? :S

    Ok thanks for the reply. Here is the whole getBest() function:
    (leafs is a vector)

    Code:
    Node3 Uct::getBest()
    {
    	int size = (int)leafs.size();
    	if(leafs.size() < 1) 
    	{
    		printf("ERROR!!!!!!!  leafs.size = 0");
    	}
    	int bestIndex = 0;
    	float bestLower = leafs[bestIndex].sampleValue - (getLipschitz()*(leafs[bestIndex].biggestInterval/2));
    	for(int i = 1; i < size; i++) 
    	{
    		float lowerBound = leafs[i].sampleValue - (getLipschitz()*(leafs[i].biggestInterval/2));
    		if(lowerBound < bestLower)
    		{
    			bestLower = lowerBound;
                            bestIndex = i;
    		}
    	}
    
    	std::cout<<"inside getBest: best parent sample: "<<(*(leafs[bestIndex].parent)).sampleValue<<std::endl;
    	std::cout<<"inside getBest: best parent memory: "<<(leafs[bestIndex].parent)<<std::endl;
    	return leafs[bestIndex];
    }

  4. #4
    Join Date
    Nov 2006
    Location
    Essen, Germany
    Posts
    1,344

    Re: Function only returns part of the object? :S

    You stopped halfway. When the leaf vector is empty you must not evaluate the item at index 0 as you do in this line of code:

    Code:
    int bestIndex = 0;
    float bestLower = leafs[bestIndex].sampleValue - (getLipschitz()*(leafs[bestIndex].biggestInterval/2));
    Use your debugger to see what nodes you are working on.

    PS:
    Please post the Node3 code. Since you return a Node3 object by value it might have something to do with the copy construction/assignment of Node3 objects.
    Last edited by GNiewerth; May 25th, 2009 at 04:18 AM.
    - Guido

  5. #5
    Join Date
    Apr 2008
    Posts
    725

    Re: Function only returns part of the object? :S

    Quote Originally Posted by cbeginner88 View Post

    I'm calling a function to find the best Node in a vector of leaf nodes and to return it.
    as said above, actually your function returns a copy of the node.

  6. #6
    Join Date
    May 2009
    Posts
    22

    Re: Function only returns part of the object? :S

    Quote Originally Posted by GNiewerth View Post
    You stopped halfway. When the leaf vector is empty you must not evaluate the item at index 0 as you do in this line of code:

    Code:
    int bestIndex = 0;
    float bestLower = leafs[bestIndex].sampleValue - (getLipschitz()*(leafs[bestIndex].biggestInterval/2));
    Use your debugger to see what nodes you are working on.
    You are right, i should have made it an if else, but the leafs vector never becomes empty. I just added the if size < 1 statement while debugging.

    Quote Originally Posted by GNiewerth View Post
    PS:
    Please post the Node3 code.
    This is the Node3 definition:

    Code:
    struct Node3 {
    	std::vector<float> sampleParam;
    	std::vector<Node3> children;
    	Node3 *parent;
    	float sampleValue;
    	float childrenMean;
    	float biggestInterval;
    	int index;
    	void test(std::vector<Node3> *leafs);
    
    	std::vector<int> splitBy; //0 to N-1	
    	std::vector<float> bounds;
    	
    	void divideInterval();
    	void divideInterval2(std::vector<Node3> *leafs);
    	float getLowerBound();
    	float getBiggestInterval();
    	float setSample();
    	float getCost(std::vector<float> param);
    	Node3() : sampleValue(NULL), childrenMean(NULL){};
    
    };
    Quote Originally Posted by GNiewerth View Post
    Since you return a Node3 object by value it might have something to do with the copy construction/assignment of Node3 objects.
    Returning a copy of the Node3 struct should be fine for my program. But i didn't explicitly write anything for copy assignments in de definition of the struct. Am i supposed to do that?

  7. #7
    Join Date
    Nov 2006
    Location
    Essen, Germany
    Posts
    1,344

    Re: Function only returns part of the object? :S

    Quote Originally Posted by cbeginner88 View Post
    Returning a copy of the Node3 struct should be fine for my program. But i didn't explicitly write anything for copy assignments in de definition of the struct. Am i supposed to do that?
    No, the Node3 struct looks fine. But it´s huge, did you consider returning an iterator/pointer to the matching node instead of returning a copy?
    Did you use a debugger to inspect the memory parent is pointing to? Does it alter when you leave getBest() scope?
    - Guido

  8. #8
    Join Date
    Apr 1999
    Posts
    27,449

    Re: Function only returns part of the object? :S

    Quote Originally Posted by cbeginner88 View Post
    Returning a copy of the Node3 struct should be fine for my program.
    What about this?
    Code:
    struct Node3 {
    	std::vector<float> sampleParam;
    	std::vector<Node3> children;
    Node3 *parent;
    The item in red is the issue. When you make a copy, how do you copy "parent"? If you copy, you now have two nodes that have the same "parent" poiinter value if you do not define a user-defined copy constructor and assignment operator.

    But i didn't explicitly write anything for copy assignments in de definition of the struct. Am i supposed to do that?
    One complaint that I have is that a lot of new programmers are being taught these concepts of classes and pointers, and almost always the case, they are never taught the responsibilities and consequences of using classes that have pointers as members.

    When you use the compiler-defined copy and assignment, the compiler makes memberwise copies of all of the members. In other words, your vectors are safe, since they themselves are copyable, however pointers are not safe. All that will be done is that the pointer value is copied from one object to another. Depending on how you handle that pointer, your code could be all screwed up if those pointers aren't handled correctly when copy/assignment is done.

    Here is a simple example:
    Code:
    class foo
    {
        int *p;
        public:
             foo() { p = new int [10]; }
            ~foo() { delete [] p; }
    };
    
    int main()
    {
       foo f;
       foo f2 = f;
       foo f3;
       f3 = f;
    }  // runtime error -- memory leaks / deleting same pointer value twice
    The simple program shows what happens when copies and assignment are made on the class that contains a pointer to dynamically allocated memory.

    So what happens if I wrote a tiny program that uses your Node class, similar to the one above? Will it behave correctly?
    Code:
    int main()
    {
        Node n;
        // assume that n has been set up
        //....
        Node n2 = n;
    }   // so what happens here when n2 and n are destroyed??
    So yes, you need to either write a user-defined copy constructor or assignment operator, or you make these functions private and unimplemented, so that the compiler will inform you with an error when a copy is attempted in the code.

    These simple concepts should always be taught, along with creation of classes with pointers, but it seems this isn't being done.

    Regards,

    Paul McKenzie

  9. #9
    Join Date
    May 2009
    Posts
    22

    Re: Function only returns part of the object? :S

    No, im not sure if i have a debugger :P. I'm just using cout to debug and print the memory address, and it remained unchanged.

    @paul mckenzie: I see what you mean. I'm not sure how it's exactly causing this problem though. But i'll definitely need to keep that in mind when using pointers in classes.

    I think that returning a pointer to the Node3 instead of a copy, should also work and actually be more efficient. But i'll need to think about that a little more to be sure it's possible here.

    Thanks for all the help and info everyone.

  10. #10
    Join Date
    Apr 1999
    Posts
    27,449

    Re: Function only returns part of the object? :S

    Quote Originally Posted by cbeginner88 View Post
    No, im not sure if i have a debugger :P. I'm just using cout to debug and print the memory address, and it remained unchanged.
    You need to use the debugger. It is one of the tools that programmers must learn to use anytime they are writing anything non-trivial.

    Regards,

    Paul McKenzie

  11. #11
    Join Date
    Apr 1999
    Posts
    27,449

    Re: Function only returns part of the object? :S

    Quote Originally Posted by cbeginner88 View Post
    @paul mckenzie: I see what you mean. I'm not sure how it's exactly causing this problem though.
    Regardless, does you Node3 object behave correctly on copying? If copying doesn't behave correctly and copying is being done, that needs to be fixed.

    Since we have no idea how you're allocating, initializing, or destroying "parent" Node3 objects in your class, the assumption is that the program subsequently will not behave correctly when you return Node3 objects by value.

    Regards,

    Paul McKenzie

  12. #12
    Join Date
    May 2009
    Posts
    22

    Re: Function only returns part of the object? :S

    It's actually never destroyed. It's part of a tree in which nodes are added. During initilization of a Node3, the parent is assigned to another existing Node3 object, which in turn in never destroyed.

    I think I don't want it to return by value at all. using return by value I think im getting a lot of small trees instead of 1 big one. So i'll need to change the getBest() return.

    And I guess i should look into debuggers

  13. #13
    Join Date
    Apr 1999
    Posts
    27,449

    Re: Function only returns part of the object? :S

    Quote Originally Posted by cbeginner88 View Post
    It's actually never destroyed. It's part of a tree in which nodes are added. During initilization of a Node3, the parent is assigned to another existing Node3 object, which in turn in never destroyed.
    It has to be destroyed at some point, or you would have a memory leak. This goes back to my first post -- it sounds like your class doesn't behave correctly on copying/assignment.
    I think I don't want it to return by value at all. using return by value I think im getting a lot of small trees instead of 1 big one.
    If you did write the copy constructor/assignment op, this wouldn't be a problem (if it's written correctly).

    So i'll need to change the getBest() return.
    And to ensure copies are not being done, do as I stated previously -- the copy constructor and assignment operator need to be private functions and unimplemented:
    Code:
    class Node3
    {
      //...
       private:
            Node3(const Node3& );
            Node3& operator =(const Node3&);
    };
    If you added these to your class, you will see that the compiler (or linker, depending on who is doing the copying) will now generate an error on your attempt to return by value.

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; May 25th, 2009 at 06:59 AM.

  14. #14
    Lindley is offline Elite Member Power Poster
    Join Date
    Oct 2007
    Location
    Seattle, WA
    Posts
    10,895

    Re: Function only returns part of the object? :S

    My guess would be that if A is in the children vector of B, then A->parent == B. That's my interpretation of the code.

    Although---where is the root node allocated? That's the only one you need to make sure is properly cleaned up.

    Since the Node3s are being stored in a vector, you can't simply disable copying on them, unfortunately. So you do need to test that and make sure it works.

  15. #15
    Join Date
    May 2009
    Posts
    22

    Re: Function only returns part of the object? :S

    Why do i need to clean up the root node? The moment that I don't need it anymore, is the moment that the program ends right?

    The error is now fixed by returning a reference to a node in getBest() instead of a copy. It now runs correctly until i have about 70 leaf nodes and then i get a segmentation fault. I forgot that pushing nodes in a vector actually copies them. So that may be the cause of the segmentation fault i think.

    I get the segmentation fault when using the push_back function of a vector. Could it be that too much memory is being used and it can't store anything else? or does that have another specific error?

    I'm gonna try to store pointers to the nodes in a vector, instead of copies and see if that fixes it. Although I find it strange that the program can get out of memory when having a tree with only 70 leaf nodes, even though there might be unneccesary copies of nodes in memory.

Page 1 of 4 1234 LastLast

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured