Custom list of objects and the warning "undeleted objects left"

New comment
 

Hi everybody,

I'm working in a project wich I created a custom object list, but after close de EA I get the message "...undeleted objects left".

My code is very big and complex, so I created a very simple one with the same logic, so, I can reproduce the exact same error and I can share the situation.

In this example code, I have a simple class that just separate the hours, minutes and seconds taken from a datetime value. This class is used in a list of objects of this same tipe, only the most important methods are available.

When the EA is closed, I have some objects left in the memory and I can not find what is missing in my code. Aparently the problem is in the method "GetListOfTimes()", because the quantity of objects left in the memory is exactly the same of the executions of the line

tList.AddElement(new CTimeUtil());


Bellow the example code:

//+------------------------------------------------------------------+
//|                                            TestingObjectTime.mq5 |
//|                                                          Gustavo |
//|                                                                  |
//+------------------------------------------------------------------+
#property copyright "Gustavo"
#property version   "1.00"

#define MAX_EXECUTIONS  2     // Maximun quantities of executions (in Timer event).
int nExec = 0;                // Execution counter.

//+------------------------------------------------------------------+
//| CLASS CTimeUtil                                                  |
//+------------------------------------------------------------------+
// This is a simple class to separate the parts of the time.
class CTimeUtil
{
   private:
      int            m_hours;
      int            m_minutes;
      int            m_seconds;
      datetime       m_dateTime;
      
   public:
                     CTimeUtil();
                     ~CTimeUtil();
      void           SetTime(datetime time);
      int            GetHours();
      int            GetMinutes();
      int            GetSeconds();
};

//+------------------------------------------------------------------+
//| CTimeUtil()                                                      |
//+------------------------------------------------------------------+
CTimeUtil::CTimeUtil()
{
   SetTime(TimeLocal());
}

//+------------------------------------------------------------------+
//| ~CTimeUtil()                                                     |
//+------------------------------------------------------------------+
CTimeUtil::~CTimeUtil()
{
}

/*+------------------------------------------------------------------+
//| SetTime()                                                        |
//+------------------------------------------------------------------+
   Define the time.
*/
void CTimeUtil::SetTime(datetime time)
{
   MqlDateTime stcTime;
   
   m_dateTime = time;
   TimeToStruct(m_dateTime, stcTime);
   m_hours = stcTime.hour;
   m_minutes = stcTime.min;
   m_seconds = stcTime.sec;
}

/*+------------------------------------------------------------------+
//| GetHours()                                                       |
//+------------------------------------------------------------------+
   Return the quantity of hours.
*/
int CTimeUtil::GetHours()
{
   return m_hours;
}

/*+------------------------------------------------------------------+
//| GetMinutes()                                                     |
//+------------------------------------------------------------------+
   Return the quantity of minutes.
*/
int CTimeUtil::GetMinutes()
{
   return m_minutes;
}

/*+------------------------------------------------------------------+
//| GetSeconds()                                                     |
//+------------------------------------------------------------------+
   Return the quantity of seconds.
*/
int CTimeUtil::GetSeconds()
{
   return m_seconds;
}


//+------------------------------------------------------------------+
//| CLASS CTimeList                                                  |
//+------------------------------------------------------------------+
// This is a class to manage a list of objects of CTimeUtil.
class CTimeList
{
   private:
      CTimeUtil      m_timeList[];
      
   public:
                     CTimeList();
                     ~CTimeList();
      void           AddElement(CTimeUtil &time);
      void           AddElement(CTimeUtil *time);
};

//+------------------------------------------------------------------+
//| CTimeList()                                                      |
//+------------------------------------------------------------------+
CTimeList::CTimeList()
{
}

//+------------------------------------------------------------------+
//| ~CTimeList()                                                     |
//+------------------------------------------------------------------+
CTimeList::~CTimeList()
{
}

/*+------------------------------------------------------------------+
//| AddElement()                                                     |
//+------------------------------------------------------------------+
   Add an element to end of the list using the referente to the object.
*/
void CTimeList::AddElement(CTimeUtil &time)
{
   AddElement(GetPointer(time));
}

/*+------------------------------------------------------------------+
//| AddElement()                                                     |
//+------------------------------------------------------------------+
   Add an element to end of the list using the pointer to the object.
*/
void CTimeList::AddElement(CTimeUtil *time)
{
   ArrayResize(m_timeList, ArraySize(m_timeList)+1);
   m_timeList[ArraySize(m_timeList)-1] = time;
}


//+------------------------------------------------------------------+
//    MAIN PROGRAM
//+------------------------------------------------------------------+


CTimeList   *myTimeList;      // Global list of objects.

//+------------------------------------------------------------------+
//| Expert initialization function                                   |
//+------------------------------------------------------------------+
int OnInit()
{
   EventSetTimer(5);
   
   printf("Stating the object list test...");
   
   return(INIT_SUCCEEDED);
}

//+------------------------------------------------------------------+
//| Expert deinitialization function                                 |
//+------------------------------------------------------------------+
void OnDeinit(const int reason)
{
   EventKillTimer();
   
   // Deleting the global object.
   if(CheckPointer(myTimeList) != POINTER_INVALID){
      delete myTimeList;
   }
   
   printf("Test finished!");
}

//+------------------------------------------------------------------+
//| Timer function                                                   |
//+------------------------------------------------------------------+
void OnTimer()
{
   // Deleting the global object before using it again.
   if(CheckPointer(myTimeList) != POINTER_INVALID){   
      delete myTimeList;
   }
   
   myTimeList = GetListOfTimes();
   
   nExec++;
   if(nExec >= MAX_EXECUTIONS){
      ExpertRemove();
   }
}

/*+------------------------------------------------------------------+
//| GetListOfTimes()                                                 |
//+------------------------------------------------------------------+
   This method represents the main task of this issue.
   The quantity of objects added in the list is exactly the same
quantity of objects left in the memory after the EA ends. This quantity
is multiplied by the times it is executed.
*/
CTimeList *GetListOfTimes()
{
   CTimeList *tList = new CTimeList;
   
   tList.AddElement(new CTimeUtil());
   tList.AddElement(new CTimeUtil());
   tList.AddElement(new CTimeUtil());
   tList.AddElement(new CTimeUtil());
   
   return GetPointer(tList);
}

//+------------------------------------------------------------------+


Thanks in advance!

 

Where are the objects created here removed from memory ?

CTimeList *GetListOfTimes()
{
   CTimeList *tList = new CTimeList;
   
   tList.AddElement(new CTimeUtil());
   tList.AddElement(new CTimeUtil());
   tList.AddElement(new CTimeUtil());
   tList.AddElement(new CTimeUtil());
   
   return GetPointer(tList);
}

They are not, and they can't because you don't have any reference to them. 

void CTimeList::AddElement(CTimeUtil *time)
{
   ArrayResize(m_timeList, ArraySize(m_timeList)+1);          // A new object is created here 
   m_timeList[ArraySize(m_timeList)-1] = time;                // then this new object is assigned with the object pointed by [time]
}

As m_timeList is an array of objects and not of pointers, AddElement creates a copy. Your reference to [time] object is lost.

There is a flaw in your code logic, you are mixing objects and pointers. Either you need to declare m_timelist as an array of pointers, or in GetListOfTimes() keep a reference to the objects created and delete them.

 

Alain is correct, your code is confusing the use of static and dynamically allocated objects and how to store/pass them. Since this is a pretty common design pattern in MQL, I'd suggest you stick with MQL libraries and common patterns. In this case you'd want to make your CTimeUtil class inherit CObject so that you can add pointers to the dynamically allocated objects to a CArrayObj vector. The CArrayObject class automatically destroys dynamic objects from its detructor by default. Think of it like a collection with a garbage collector.


Here is a quick example of how to do that. 


#include <arrays/arrayobj.mqh>

template<typename T>
class CObjectVector : public CArrayObj {
    public: T operator[](const int index) const { return this.At(index); }
};


class CTimeUtil : public CObject {
 private:
   MqlDateTime       m_time;
 public:
                     CTimeUtil(){ TimeToStruct(TimeCurrent(), m_time); }
   int               hour()     { return m_time.hour; }
   int               min()      { return m_time.min;  }
   int               sec()      { return m_time.sec;  }
};


void OnTick() {
    static CObjectVector<CTimeUtil*> time_vector();
    time_vector.Add(new CTimeUtil());
    timeVectorOnChart(time_vector);
}

void timeVectorOnChart(CObjectVector<CTimeUtil*> &time_vector) {
    string res_str = StringFormat("ArraySize(%d)", time_vector.Total());
    for (int i=time_vector.Total()-1, j=0; i>=0 && j<20; i--, ++j) {
        CTimeUtil *time = time_vector[i];
        res_str += StringFormat(
            "\n(%d, %d, %d)", 
            time.hour(), 
            time.min(), 
            time.sec()
        );
    }
    Comment(res_str);
}
 

Thanks for the tips guys.

Alain Verleyen:

Where are the objects created here removed from memory ?

I thought that MQL would automatically remove, this was my error.


Updating the code and keeping the reference, this way works, but I liked the @nicholi shen' advice:

CTimeList *GetListOfTimes()
{
   CTimeList *tList = new CTimeList;
   CTimeUtil tUtil;
   
   tUtil.SetTime(TimeLocal());
   tList.AddElement(tUtil);
   
   tUtil.SetTime(TimeLocal());
   tList.AddElement(tUtil);
   
   tUtil.SetTime(TimeLocal());
   tList.AddElement(tUtil);
   
   return GetPointer(tList);
}
 
nicholi shen:

Alain is correct, your code is confusing the use of static and dynamically allocated objects and how to store/pass them. Since this is a pretty common design pattern in MQL, I'd suggest you stick with MQL libraries and common patterns. In this case you'd want to make your CTimeUtil class inherit CObject so that you can add pointers to the dynamically allocated objects to a CArrayObj vector. The CArrayObject class automatically destroys dynamic objects from its detructor by default. Think of it like a collection with a garbage collector.


Here is a quick example of how to do that. 


Thanks,

I'll try it.

New comment
Reason: