[Closed] Bug mql4 build 616 - Static class members in global scope not visible to expert functions

New comment
 

Just in case anyone is playing with Singletons in MQL4++ and failing (like me), I've reported the following at the service desk (Fairly sure it's a compiler bug...)

EDIT: Posted here for visibility to other developers. Will update thread when fixed.


// This expert demonstrates that static members in the global scope are 'different' to those accessed via the Expert functions
// In this case the static member StaticInt::v will be set , when called from the globalScope, but this will not be visible
// when accessed via the expert function (OnInit,OnTick,OnDeinit)
 
#property strict

// This class just holds a static value 'v'. 
// If it were a real Singleton then v would be a pointer to the Single Instance     
class StaticInt {
 private:
 static int v;
  
 public:
 static void getAndSet(string msg,int newVal) {
   Print(StringConcatenate(msg," was ",v," now ",newVal));
   v = newVal;
 }
 
};

// This class is just used to call StaticInt::getAndSet() at global scope.
class Updater {
   public:
   Updater() {
      StaticInt::getAndSet("via UpdaterConstructor",111);
   }
};

// Call StaticInt::getAndSet() twice at global level via default constructor of Updater  
Updater tmp[2];


int OnInit()
  {
      StaticInt::getAndSet("OnInit1:",1001);
      StaticInt::getAndSet("OnInit2:",1002);
      return(INIT_SUCCEEDED);
  }

void OnDeinit(const int reason)
  {
      StaticInt::getAndSet("OnDenit1:",2001);
      StaticInt::getAndSet("OnDenit2:",2002);
  }

void OnTick()
  {
  }

Output: '0' should be '111'

01:54:11 Expert static2 EURUSD,M1: removed
01:54:11 Expert static2 EURUSD,M1: loaded successfully
01:54:11 TestGenerator: spread set to 20
01:54:14 static2 test started
01:54:14 2013.07.10 00:00 static2 EURUSD,M1: via UpdaterConstructor was 0 now 111
01:54:14 2013.07.10 00:00 static2 EURUSD,M1: via UpdaterConstructor was 111 now 111
01:54:14 2013.07.10 00:00 static2 EURUSD,M1: OnInit1: was 0 now 1001
01:54:14 2013.07.10 00:00 static2 EURUSD,M1: OnInit2: was 1001 now 1002
01:54:14 2013.07.16 23:59 static2 EURUSD,M1: OnDenit1: was 1002 now 2001
01:54:14 2013.07.16 23:59 static2 EURUSD,M1: OnDenit2: was 2001 now 2002
01:54:14 2013.07.16 23:59 static2 EURUSD,M1: 2 undeleted objects left
01:54:14 2013.07.16 23:59 static2 EURUSD,M1: 2 objects of type Updater left
01:54:14 EURUSD,M1: 397152 tick events (8181 bars, 398152 bar states) processed within 62 ms (total time 2605 ms


Also aren't dynamically allocated arrays of objects supposed to be released/GC'd when the array is released? Or have I mis-read Creating and Deleting Objects ? Will highlight in another thread..here


 

Here is the response from support desk:

We have implemented changes in the compiler.

We have decided not to define static variables by the compiler. Now, a developer must do that instead. ie 

#property strict

// This class just holds a static value 'v'. 
// If it were a real Singleton then v would be a pointer to the Single Instance     
class StaticInt {
 private:
 static int v;
  
 public:
 static void getAndSet(string msg,int newVal) {
   Print(StringConcatenate(msg," was ",v," now ",newVal));
   v = newVal;
 }
 
};

// This class is just used to call StaticInt::getAndSet() at global scope.
class Updater {
   int i;
   public:
   Updater() {
      StaticInt::getAndSet("via UpdaterConstructor",111);
   }
};

// Call StaticInt::getAndSet() at global level via default constructor of Updater  

int StaticInt::v=0;

Updater tmp;


int OnInit()
  {
      StaticInt::getAndSet("OnInit1:",1001);
      StaticInt::getAndSet("OnInit2:",1002);
      return(INIT_SUCCEEDED);
  }

void OnDeinit(const int reason)
  {
      StaticInt::getAndSet("OnDenit1:",2001);
      StaticInt::getAndSet("OnDenit2:",2002);

  }

void OnTick()
  {

  }

Output:

16:04:35 Expert static EURUSD,M1: loaded successfully
16:04:35 TestGenerator: spread set to 20
16:04:38 static test started
16:04:38 2013.07.10 00:00 static EURUSD,M1: via UpdaterConstructor was 0 now 111
16:04:38 2013.07.10 00:00 static EURUSD,M1: OnInit1: was 111 now 1001
16:04:38 2013.07.10 00:00 static EURUSD,M1: OnInit2: was 1001 now 1002
16:04:38 2013.07.16 23:59 static EURUSD,M1: OnDenit1: was 1002 now 2001
16:04:38 2013.07.16 23:59 static EURUSD,M1: OnDenit2: was 2001 now 2002
16:04:38 EURUSD,M1: 397152 tick events (8181 bars, 398152 bar states) processed within 62 ms (total time 3136 ms)


I was skeptical of the 'fix' above but it works. It does means some of the documentation is wrong (or at least needs some clarification)

It is not required to initialize static class members explicitly at the global level, they will be initialized automatically when you start the program. The static member of the integer and real type is automatically initialized with zero, to type string NULL is assigned.



 

In addition to changing the documentation the compiler should really warn here.

BTW. This is a good reason to use singleton instances rather than static classes (classes with lots of static members) - which is considered an anti-pattern.

Only one static member of the class to make sure it is initialized correctly.

 

I did not even understand what was going on in the fix :)

Just a note about your singletons. It is not a singleton pattern as I know. I use singleton pattern only for objects (rather than simple variables) - like the class in the following example: 

class Logger {
private:
   static Logger* logger;

   /** 
    * Private constructor, use getLogger() instead of new() .
    */
   Logger() {}

public:
   /** 
    * Returns logger singleton.
    */
   static Logger* getLogger() {
      if (logger == NULL) {
         logger = new Logger();
      }
      return (logger);
   }

   /** 
    * Destructs logger
    */
   static void closeLogger() {
      if(logger != NULL) {
         delete(logger);
         logger = NULL;
      }
   }

   //list of object methods follows --------

   void doSomething() {
       // it does something
   }
};
 

Hi yes. The singleton pattern is for objects. And the static variable would be an object pointer. I only used int in the test case to make the test case simpler.

Sorry for any confusion!

So all my singleton classes look like this now:

class MySingleton {

    static MySingleton* INSTANCE;

    ///private constructor use getInstance()
    MySingleton() { ... }

    public:
    /// Get single instance
     static MySingleton *getInstance() {
        if (INSTANCE == NULL) {
            INSTANCE = new MySingleton();
        }
        return INSTANCE;
    }

    ///Release object

     static void destroy() {
        if (INSTANCE != NULL) {
            delete INSTANCE;
            INSTANCE = NULL;
        }
    }

    ...
};
// Must initialize static members to avoid 'bug' even though initial value is NULL
MySingleton *MySingleton::INSTANCE = NULL;

With singletons there is only one static class member to initialise so it makes this mql 'bug' easier to manage.

I keep the initialization immediately after the class definition in the same file/module.

[EDIT: Minor improvement. The destructor should be made private and/or the INSTANCE should be cleared in the destructor itself. ]

New comment
Reason: