Having trouble identifying trouble in alarm trigger code

 

Hi Everyone.

I have been coding for afew months now, however seem to be having difficulty with this. I have had a go at it for a couple of weeks now on and off, but to no avail.

I simply want an alarm, to go off when the two indicator lines cross, every new bar, however I must be overlooking something as at the moment, it would appear that it is giving me an alarm every minute. I have tried changing many aspects of the code, but so far have not been able to distinguish which part of the code is giving me problems.

Any suggestions would be appreciated.

Thanks for your time.

//---- indicator settings
#property  indicator_separate_window
#property  indicator_buffers 2
#property  indicator_color1  Yellow
#property  indicator_color2  Red
#property  indicator_width1  1

//---- indicator parameters
extern string PairName = "";   // Leave blank for the pair of the chart, enter other pair name to compare correlated pairs

extern int StdDev.MA.Period=12;  // D1=20
extern int StdDev.MA.Shift=0;    //
extern int StdDev.MA.Method = 0; // 0=SMA 1=EMA 2=Smoothed 3=Linear Weighted
extern int StdDev.MA.Price = 0;  // 0 Close price, 1 Open price, 2 High price, 3 Low price, 4 Median price, (high+low)/2, 5 Typical price, (high+low+close)/3, 6 Weighted close price, (high+low+close+close)/4

extern int MA.Fast.Period = 3;
extern int MA.Fast.Method = 2;   //  0=SMA 1=EMA 2=Smoothed 3=Linear Weighted
extern int MA.Fast.Shift = 0;

extern bool CheckOncePerBar = true;

int i, limit, counted_bars;
static string Pair1;

datetime CurrentTimeStamp;

//---- indicator buffers
double     STDBuffer[];
double     stddevma[];
double     LastSTDBuffer[];
double     Laststddevma[];

//+------------------------------------------------------------------+
//| Custom indicator initialization function                         |
//+------------------------------------------------------------------+
int init()
  {
  
   IndicatorDigits(Digits+1);
     
//---- drawing settings
   SetIndexStyle(0,DRAW_LINE); // 
   SetIndexStyle(1,DRAW_LINE);

      
//---- indicator buffers mapping
   SetIndexBuffer(0, STDBuffer);
   SetIndexBuffer(1, stddevma);

   
   if (PairName == "") Pair1 = Symbol();
   else Pair1 = PairName;

//---- name for DataWindow and indicator subwindow label
   IndicatorShortName("SFX TOR: "+Pair1+"("+StdDev.MA.Period+")");
   SetIndexLabel(0,"StdDev");
   SetIndexLabel(1,"StdDev MA");

//---- initialization done
   return(0);
  }
//+------------------------------------------------------------------+
//| Moving Averages Convergence/Divergence                           |
//+------------------------------------------------------------------+
int start()
  {

   counted_bars=IndicatorCounted();
   
//---- last counted bar will be recounted
   if(counted_bars>0) counted_bars--;
   limit=Bars-counted_bars;
   
//---- macd counted in the 1-st buffer
   for(i=0; i<limit; i++)
    {
      STDBuffer[i]=iStdDev(Pair1,0,StdDev.MA.Period, StdDev.MA.Shift, StdDev.MA.Method, StdDev.MA.Price, i);
    }
    
   for(i=0; i<limit; i++)   
    {

     stddevma[i] = iMAOnArray(STDBuffer, 0, MA.Fast.Period, MA.Fast.Shift, MA.Fast.Method, i);

    }
    
   for(i=0; i<limit; i++)
    {
      LastSTDBuffer[i]=iStdDev(Pair1,0,StdDev.MA.Period, StdDev.MA.Shift, StdDev.MA.Method, StdDev.MA.Price, -1);
    }
    
   for(i=0; i<limit; i++)   
    {

     Laststddevma[i] = iMAOnArray(STDBuffer, 0, MA.Fast.Period, MA.Fast.Shift, MA.Fast.Method, i-1);

    }
    
//Execute on bar Open
   if(CheckOncePerBar == true)
      {
         int BarShift = 1;
         if (CurrentTimeStamp != Time[0])
            {
               CurrentTimeStamp = Time [0];
               bool NewBar = true;
            }
          else NewBar = false;
      }
        
   if(NewBar == true && STDBuffer[i] > stddevma[i] && LastSTDBuffer[i] <= Laststddevma[i])
      {
         PlaySound("alert.wav");
         Alert(Symbol()," M",Period()," Crossing");
         
      }
         
   if(NewBar == true && STDBuffer[i] < stddevma[i] && LastSTDBuffer[i] >= Laststddevma[i])
      {
         PlaySound("alert.wav");
         Alert(Symbol()," M",Period()," Crossing");
         
      }
      
   

          
//---- done
   return(0);
  }

Mike

 
//Execute on bar Open
What is the value of "i" below that line, since you are OUTSIDE of the loops.
 

Are you refering to the "i" that is after the names of the lines? Such as stddevma[i]? And the rest?
This is where I am getting confused, how to I get these to relate to the variables that should be being stored in the buffers at the start of the code?
As I said I am reasonably new and I would imagine this is a 'rookie' mistake but I am willing to listen and learn if you could explain it to me.

Thanks

 

You have 4 separate for loops all doing the same thing, why repeat the same thing 4 times ? why not just do it once ?

WHRoeder is referring to this code . . . what is the value of i in this code ?

//Execute on bar Open
   if(CheckOncePerBar == true)
      {
         int BarShift = 1;
         if (CurrentTimeStamp != Time[0])
            {
               CurrentTimeStamp = Time [0];
               bool NewBar = true;
            }
          else NewBar = false;
      }
        
   if(NewBar == true && STDBuffer[i] > stddevma[i] && LastSTDBuffer[i] <= Laststddevma[i])   // <---  i  ??
      {
         PlaySound("alert.wav");
         Alert(Symbol()," M",Period()," Crossing");
         
      }
         
   if(NewBar == true && STDBuffer[i] < stddevma[i] && LastSTDBuffer[i] >= Laststddevma[i])   // <--  i  ??
      {
         PlaySound("alert.wav");
         Alert(Symbol()," M",Period()," Crossing");
         
      }
 

Thanks for your reply.

I am trying to get "i" in each one to refer to the buffer for each one. However removing the "i' causes a huge amount of errors when I go to compile.
I am struggling to get the indicator to recognise what's in the buffers, from what I can tell.
As I said, im new, and any helpful advice is very very much welcome.

Mike

 

What problems are you getting? I'm assuming that you are looking for the latest values of your indicator so you should replace all the "i" with 0's:

if(NewBar == true && STDBuffer[0] > stddevma[0] && LastSTDBuffer[0] <= Laststddevma[0])
 

Let's break it down a bit. Consider this section of code ...

   for(i=0; i<limit; i++)
    {
      STDBuffer[i]=iStdDev(Pair1,0,StdDev.MA.Period, StdDev.MA.Shift, StdDev.MA.Method, StdDev.MA.Price, i);
    }
    
   for(i=0; i<limit; i++)   
    {

     stddevma[i] = iMAOnArray(STDBuffer, 0, MA.Fast.Period, MA.Fast.Shift, MA.Fast.Method, i);

    }
    
   for(i=0; i<limit; i++)
    {
      LastSTDBuffer[i]=iStdDev(Pair1,0,StdDev.MA.Period, StdDev.MA.Shift, StdDev.MA.Method, StdDev.MA.Price, -1);
    }
    
   for(i=0; i<limit; i++)   
    {

     Laststddevma[i] = iMAOnArray(STDBuffer, 0, MA.Fast.Period, MA.Fast.Shift, MA.Fast.Method, i-1);

    }


There are 4 loops all doing the same thing, as Raptor has mentioned. So we combine them to get ...

   for(i=0; i<limit; i++){

     STDBuffer[i]=iStdDev(Pair1,0,StdDev.MA.Period, StdDev.MA.Shift, StdDev.MA.Method, StdDev.MA.Price, i);
     stddevma[i] = iMAOnArray(STDBuffer, 0, MA.Fast.Period, MA.Fast.Shift, MA.Fast.Method, i);
     LastSTDBuffer[i]=iStdDev(Pair1,0,StdDev.MA.Period, StdDev.MA.Shift, StdDev.MA.Method, StdDev.MA.Price, -1);
     Laststddevma[i] = iMAOnArray(STDBuffer, 0, MA.Fast.Period, MA.Fast.Shift, MA.Fast.Method, i-1);

   } 

Now we can see what's going on it is clear you are doing the same thing for two of the buffers, but shifted by one point. But all the data is already in the other buffer anyway so there is no need to duplicate the data. If you want to refer to bar N and the bar before that, which is N+1 that's all you need to do. So scrap those Last... buffers.

And I have done some other tweaks. I have made the index counter i local rather than global. (Delete the declaration of i outside of the function. I am counting down in the loop rather than up. That is how all the examples are done.

   for(int i=limit-1; i>=0; i--){
     STDBuffer[i]=iStdDev(Pair1,0,StdDev.MA.Period, StdDev.MA.Shift, StdDev.MA.Method, StdDev.MA.Price, i);
   }

   for(int i=limit-1; i>=0; i--){
     stddevma[i] = iMAOnArray(STDBuffer, 0, MA.Fast.Period, MA.Fast.Shift, MA.Fast.Method, i);
   }  

So we have filled the buffers with data. Next we get onto your tests.

 

So let's look at the next part ...

   if(CheckOncePerBar == true)
      {
         int BarShift = 1;
         if (CurrentTimeStamp != Time[0])
            {
               CurrentTimeStamp = Time [0];
               bool NewBar = true;
            }
          else NewBar = false;
      }
        
   if(NewBar == true && STDBuffer[i] > stddevma[i] && LastSTDBuffer[i] <= Laststddevma[i])
      {
         PlaySound("alert.wav");
         Alert(Symbol()," M",Period()," Crossing");
         
      }
         
   if(NewBar == true && STDBuffer[i] < stddevma[i] && LastSTDBuffer[i] >= Laststddevma[i])
      {
         PlaySound("alert.wav");
         Alert(Symbol()," M",Period()," Crossing");
         
      }
      

Notice that the bool variable is defined within braces and should, according to the C++ programming language, be local to those braces (in standard C all the declarations have to be at the start of the function). MQL4 does not follow either standard and allows weirdness like this :-(

Also BarShift is defined but not used. Now how we normally do this next bit is to return when we have no further work to do, rather than set a flag (NewBar) to tell us we don't want to do any more work.

   if( CheckOncePerBar == true ){
      if( CurrentTimeStamp == Time[0] )
         return( 0 );
   }
   
   CurrentTimeStamp = Time [0];

Finally we have the tests which are now simpler

 

The buffer index 1 is the bar that just closed, and index 2 is the bar before that one ...

   if( STDBuffer[1] > stddevma[1] ){
      if( STDBuffer[2] <= stddevma[2]){
         PlaySound("alert.wav");
         Alert(Symbol()," M",Period()," Crossing");
      }   
   }
   else{ // if it's not greater then it is either lesser than or equal
      if( STDBuffer[2] >= stddevma[2]){
         PlaySound("alert.wav");
         Alert(Symbol()," M",Period()," Crossing");   
      }
   }
 
dabbler:

The buffer index 1 is the bar that just closed, and index 2 is the bar before that one ...


Thanks for all your help cleaning up the code that I had and making it work! I am very happy right now thank you a lot for your help, everything is alot clearer now.

I do have one question however at the moment, each new bar, the code seems to be calculating for anywhere between half a second and about 10 seconds. When it does this it looks like the top of the image below, then it goes back to normal, as the lower graph shows. Why would it do this? I have not seen this before, I was thinking that prehaps there might be part of the code which is inefficent and takes longer to process than it should?

 
NewCoder47:

I do have one question however at the moment, each new bar, the code seems to be calculating for anywhere between half a second and about 10 seconds. When it does this it looks like the top of the image below, then it goes back to normal, as the lower graph shows. Why would it do this? I have not seen this before, I was thinking that perhaps there might be part of the code which is inefficient and takes longer to process than it should?

Post the complete code and we'll take a look.
Reason: