CList GetNodeAtIndex bug?

New comment
 

I am trying a fairly simple thing and failing miserably. Sorry for the long post but this got me scratching my head. Any explanation would be greatly appreciated.



My advisor is supposed to draw some trendlines.

I find some pivot points and store them in a CList called Pivots. No problem here.

But when I try to store those points as pairs in other CLists, the first CList starts to return "invalid pointer access" errors. (even though there is absolutely no tempering with the first CList, only GetNodeAtIndex calls)

When I investigate deeper, I find out that after the first for loop, GetNodeAtIndex returns null even though there should be something to return. (ie: GetNodeAtIndex(2) returns null on a CList with count of 6)

I worked around the issue by not storing the values of GetNodeAtIndex directly but rather storing their copies. But this doesn't make sense to me.

Should I not be using the values directly ever? Is this a memory mismanagement issue?


Am I doing something wrong here or is this a bug with CList?


Example code:

You can uncomment the parts that say (THIS WAY WORKS AS INTENDED) and comment out the (THIS DOESN'T WORK) parts to see it working. There are some prints to give an idea about whats going on.

//+------------------------------------------------------------------+
//|                                                        test2.mq5 |
//|                        Copyright 2019, MetaQuotes Software Corp. |
//|                                             https://www.mql5.com |
//+------------------------------------------------------------------+
#property copyright "Copyright 2019, MetaQuotes Software Corp."
#property link      "https://www.mql5.com"
#property version   "1.00"


#include <Arrays\List.mqh>

class TimePrice : public CObject {
public:
    uint    time;
    double  price;
};

class Line : public CObject {
    public:
        CList   *members;
        
        // Constructor
        Line() {
            members = new CList;
        };
};

int Range = 100;
int Proximity = 5;

CList *Lines;
CList *Pivots;


int OnInit() {
   
    ///////////////////////////////////////////////////////////////
    // SET PIVOTS
        
    Pivots = new CList;

    for(int i = 0; i < Range; i++) {
        int index = i - Proximity;
        if(index < 0)
            index = 0;

        int localHighIndex = iHighest(_Symbol, _Period, MODE_HIGH, Proximity*2, index);

        if(i == localHighIndex) {
            TimePrice *tp = new TimePrice;
            tp.time = iTime(_Symbol, _Period, i);
            tp.price = iHigh(_Symbol, _Period, i);

            Pivots.Add(tp);

            MarkByIndex(i, OBJ_ARROW_SELL);
        }
    }
    ///////////////////////////////////////////////////////////////
    
    
    ///////////////////////////////////////////////////////////////
    // DEFINE LINES
    
    Lines = new CList;
    
    int countPivots = Pivots.Total();
    
    for (int i = 1; i < countPivots; i++) {
        printf("i: " + i);
        
        //////////////////////////////////////////////////
        // THIS WAY WORKS AS INTENDED
        //TimePrice *tpAtemp = Pivots.GetNodeAtIndex(i);
        //TimePrice *tpA = new TimePrice;
        //tpA.time = tpAtemp.time;
        //tpA.price = tpAtemp.price;
        //////////////////////////////////////////////////
        
        //////////////////////////////////////////////////
        // THIS DOESN'T WORK
        TimePrice *tpA = Pivots.GetNodeAtIndex(i);
        //////////////////////////////////////////////////
        
        printf("A: %u %f", tpA.time, tpA.price);
            
        
        for (int j = i+1; j < countPivots; j++) {
            printf("    j: " + j);
            
            //////////////////////////////////////////////////
            // THIS WAY WORKS AS INTENDED
            //TimePrice *tpBtemp = Pivots.GetNodeAtIndex(j);
            //TimePrice *tpB = new TimePrice;
            //tpB.time = tpBtemp.time;
            //tpB.price = tpBtemp.price;
            //////////////////////////////////////////////////
            
            //////////////////////////////////////////////////
            // THIS DOESN'T WORK
            TimePrice *tpB = Pivots.GetNodeAtIndex(j);
            //////////////////////////////////////////////////
            
            printf("    B: %u %f", tpB.time, tpB.price);
        
            // New Line
            Line *nl = new Line;
            nl.members.Add(tpA);
            nl.members.Add(tpB);
            Lines.Add(nl);
            
            
            // DRAW LINE HERE
            // ...
            
            delete tpB;
        }
        
        delete tpA;
    }
    
    printf(Lines.Total());
    ///////////////////////////////////////////////////////////////
    
    return(INIT_SUCCEEDED);
}



void MarkByIndex(int index, ENUM_OBJECT objEnum) {
    datetime t = iTime(_Symbol, _Period, index);

    double price;
    if(objEnum == OBJ_ARROW_SELL)
        price = iHigh(_Symbol, _Period, index);
    else if(objEnum == OBJ_ARROW_BUY)
        price = iLow(_Symbol, _Period, index);

    ObjectCreate(0, "objName_" + objEnum + "_" + index, objEnum, 0, t, price);
}
 
If you don't want to store copies, don't delete the original objects. Why are you delete 'tpB' and 'tpA' ? These are pointers to your original objects so of course after that you get NULL.
 
Alain Verleyen:
If you don't want to store copies, don't delete the original objects. Why are you delete 'tpB' and 'tpA' ? These are pointers to your original objects so of course after that you get NULL.

Those lines don't change anyhing.

You can comment them out and get the same results.

(I accidentally deleted my last reply, so posting it again)

 
kwarmeta:

Those lines don't change anyhing.

You can comment them out and get the same results.

(I accidentally deleted my last reply, so posting it again)

You are right and wrong. That's right you get the same results (invalid pointer access)  but it's wrong it "changes anything". Because there is an other problem, using the same object in 2 different CList doesn't work. When you add tpA or tpB to your Line (internal CList) it changes the state of the object (pointer prev/next), which in turn changes the Pivots CList behaviour.

That means you HAVE TO USE a COPY, for your other list (CLine). That's not a bug but rather a limitation of CList due to a bad design. You have to deal with it.

 
Alain Verleyen:

You are right and wrong. That's right you get the same results (invalid pointer access)  but it's wrong it "changes anything". Because there is an other problem, using the same object in 2 different CList doesn't work. When you add tpA or tpB to your Line (internal CList) it changes the state of the object (pointer prev/next), which in turn changes the Pivots CList behaviour.

That means you HAVE TO USE COPY, for your other list (CLine). That's not a bug but rather a limitation of CList due to a bad design. You have to deal with it.

I see, its a bit weird. I guess I have some reading to do :)

Thanks a lot for the explanation.

 
Alain Verleyen #:

You are right and wrong. That's right you get the same results (invalid pointer access)  but it's wrong it "changes anything". Because there is an other problem, using the same object in 2 different CList doesn't work. When you add tpA or tpB to your Line (internal CList) it changes the state of the object (pointer prev/next), which in turn changes the Pivots CList behaviour.

That means you HAVE TO USE a COPY, for your other list (CLine). That's not a bug but rather a limitation of CList due to a bad design. You have to deal with it.

Great explanation. I had the same problem and didnt know that it's not possible to point from two different lists to the same object .

Thank you very much :)

New comment
Reason: