Problem with implementing a singleton pattern for multiple instances with Array

New comment
 
Hello friends
The code below has two problems that I don't understand.
1. When I make the constructor private, the compiler gives an error. Even when I define a single pointer, it gives an error without instantiation. Shouldn't we do this for singletons?!
2. The array that I have defined to store instances based on the time frames is static and I expect it not to be destroyed with the object and to remain until the end of the program, but it deletes the instance every time!
I checked the code with the CPP and DeepSiq examples, it still doesn't work
If you can help, I would be very grateful. 
class CDataSeries// : public CPriceFactory
  {
protected:

                     CDataSeries(int period_ = NULL, string symbol_ = NULL) {copied = CopyRates(symbol_, period_, 0, Bars, rate); }
public:

                    ~CDataSeries() { ArrayFree(rate); delete open; delete high; delete low; delete close;                                        }
   static  CDataSeries *instance(int period_ = NULL, string symbol_ = NULL);
   static bool       Range(int period_ = NULL, Array<double>* res = NULL,  string symbol_ = NULL);
   static bool       Body(int period_ = NULL, Array<double>* res = NULL,  string symbol_ = NULL);
   static bool       OpenToOpen(int period_ = NULL, Array<double>* res = NULL,  string symbol_ = NULL);
   static bool       HighToLow(int period_ = NULL, Array<double>* res = NULL,  string symbol_ = NULL);
   static bool       Export(int period_ = NULL, Array<double>* res = NULL,  string symbol_ = NULL);
   static bool       BodyToRange(int period_ = NULL, Array<double>* res = NULL,  string symbol_ = NULL);
   int const         size(void) const { return(copied);}
   //---
private:
   //---
   int                copied;
   MqlRates           rate[];
   Array<double>      *Open(void);
   Array<double>      *High(void);
   Array<double>      *Low(void);
   Array<double>      *Close(void);
   //--- extracted esries
   Array<double>      *open;
   Array<double>      *high;
   Array<double>      *low;
   Array<double>      *close;
   static CDataSeries *rates[8];
   static int const  period[8];
   static CDataSeries *Self;
  };
static CDataSeries* CDataSeries::rates[8];
static CDataSeries* CDataSeries::Self = NULL;
static int const CDataSeries::period[8] = {PERIOD_M1, PERIOD_M5, PERIOD_M15, PERIOD_H1, PERIOD_H4, PERIOD_D1, PERIOD_W1, PERIOD_MN1};
//+------------------------------------------------------------------+
CDataSeries* CDataSeries::instance(int period_ = NULL, string symbol_ = NULL)
  {
   if(!Self)
     {
      for(int i = 0; i < 8; i++)
        {
         if(period_ == CDataSeries::period[i])
           {
            if(!rates[i])
              {
               rates[i] = new CDataSeries(period_, symbol_);
               Self = rates[i];
               break;
              }
           }
        }
     }
   Print(rates[0], rates[1], rates[2], rates[3], rates[4], rates[5], rates[6], rates[7]);
   Print(Self);
   return(Self);
  }
//| rate of open price                                                                 |
//+------------------------------------------------------------------+
Array<double>* CDataSeries::Open(void)
  {
   if(!open)
     {
      open = new Array<double>;
      for(int i = 0; i < size(); i++)
         open.Append(rate[i].open);
     }
   return(open);
  }
//| rate of high price
//+------------------------------------------------------------------+
Array<double>* CDataSeries::High(void)
  {
   if(!high)
     {
      high = new Array<double>;
      for(int i = 0; i < size(); i++)
         high.Append(rate[i].high);
     }
   return(high);
  }
//| rate of low price
//+------------------------------------------------------------------+
Array<double>* CDataSeries::Low(void)
  {
   if(!low)
     {
      low = new Array<double>;
      for(int i = 0; i < size(); i++)
         low.Append(rate[i].low);
     }
   return(low);
  }
//| rate of close price                                            |
//+------------------------------------------------------------------+
Array<double>* CDataSeries::Close(void)
  {
   if(!close)
     {
      close = new Array<double>;
      for(int i = 0; i < size(); i++)
         close.Append(rate[i].close);
     }
   return(close);
  }
//| rate of candle range
//+------------------------------------------------------------------+
static bool CDataSeries::Range(int period_, Array<double>* res, string symbol_ = NULL)
  {
   CDataSeries *seri = CDataSeries::instance(period_, symbol_);
//---
   for(int i = 0; i < seri.size(); i++)
     {
      res.Append(seri.High()[i] - seri.Low()[i]);
     }
   return(true);
  }
//| rate of body range

 
Change the default arguments from NULL to valid defaults (e.g., 0 and "" ), and ensure that the static member function (or friend functions) has access to the private constructor. 
// Use 0 for period and an empty string for symbol
CDataSeries(int period_ = 0, string symbol_ = "") { /* ...your code... */ }


Static pointer array only has static storage; it does not control the lifetime of the objects. To prevent the object from being destroyed prematurely, do not call delete on it (or provide an explicit lifetime management strategy).

Try this Options....

 
Sivakumar Paul Suyambu #:
Change the default arguments from NULL to valid defaults (e.g., 0 and "" ), and ensure that the static member function (or friend functions) has access to the private constructor.


Static pointer array only has static storage; it does not control the lifetime of the objects. To prevent the object from being destroyed prematurely, do not call delete on it (or provide an explicit lifetime management strategy).

Try this Options....

Thank you my friend for your time, changing the constructor worked correctly. But for the array problem, how can I implement explicit lifetime? Could you give an example? Thanks again.

 

moslem alavi:

   MqlRates           rate[];
   Array<double>      *Open(void);
   Array<double>      *High(void);
   Array<double>      *Low(void);
   Array<double>      *Close(void);
   //--- extracted esries
   Array<double>      *open;
   Array<double>      *high;
   Array<double>      *low;
   Array<double>      *close;

I'm curious why you need such a cumbersome thing, instead of copying synchronized data into several separate arrays with one function call.

https://www.mql5.com/en/docs/series/copyseries

The CopySeries function allows obtaining only the necessary timeseries into different specified arrays during one call, while all of timeseries data will be synchronized. This means that all values in the resulting arrays at a certain index N will belong to the same bar on the specified Symbol/Timeframe pair. Therefore, there is no need for the programmer to ensure the synchronization of all received timeseries by the bar opening time.

Thus, if you need to get the values of the time, close and real_volume timeseries for the last 100 bars of the current Symbol/Timeframe, you should use the following call:

datetime time[];
double   close[];
long     volume[];
CopySeries(NULL,0,0,100,COPY_RATES_TIME|COPY_RATES_CLOSE|COPY_RATES_VOLUME_REAL,time,close,volume);
 
Vladislav Boyko #:

I'm curious why you need such a cumbersome thing, instead of copying synchronized data into several separate arrays with one function call.

You are absolutely right. It is very cumbersome and redundant. But unfortunately the CopySeries function does not exist in mql4. Of course, as you mentioned, there are similar built-in functions that I will use in the main code. Thank you for your attention. In any case, the main problem was in storing the instances in the static Array , which (probably was not initialized correctly and was solved by rewriting the instance function). I will leave the code sample below. Thank you

CDataSeries* CDataSeries::instance(int period_ = 0, string symbol_ = "")
  {
   for(int i = 0; i < 8; i++)
     {
      if(period_ == CPriceFactory::period[i])
        {
         if(!CDataSeries::rates[i])
            CDataSeries::rates[i] = new CDataSeries(period_, symbol_);
         return(CDataSeries::rates[i]);
        }
     }
   return(instance(period_, symbol_)); // Changed to recursive to ensure initialization
  }
 

moslem alavi #:

if(!CDataSeries::rates[i])
   CDataSeries::rates[i] = new CDataSeries(period_, symbol_);

in mql4

Implicit call to CheckPointer() using the NLOT operator does not work fully in MQL4. Therefore, in MQL4 I personally prefer to call CheckPointer explicitly

#property strict

class A
  {
public:
   int get() { return(x); }
private:
   int x;
public:
   A::A(int a_x) : x(a_x) {}
  };

void OnStart()
  {
   A* a = new A(MathRand() % 100);
   if(!a)
      Print("#1: Pointer is ", EnumToString(CheckPointer(a)));
   else Print("#1: x = ", a.get());
   delete a;
   if(!a)
      Print("#2: Pointer is ", EnumToString(CheckPointer(a)));
   else Print("#2: x = ", a.get());
  }


New comment