What's the problem with my code?

New comment
 

Dear all

 

I wrote an indicator to look for following condition:

 

If price is within ibands(deviation 1, upper) and ibands(deviation 1, lower)  for 2 timeframes, give me a "1" result.

 

However, it doesn't work when I put it on GBPUSD H1 chart.  Can anybody tell me how to fix it?

 

Thanks a lot!

 

wing

 

#property indicator_separate_window
#property indicator_buffers 3
#property indicator_color1 Gold
#property indicator_width1 2
#property indicator_color2 Green
#property indicator_width2 2
#property indicator_color3 Blue
#property indicator_width3 2
#property indicator_maximum 2.0
#property indicator_minimum -2.0
#property indicator_level1 0
//-------------------------------------
extern string     TimeFrames     = "1; 5; 15; 30; 60; 240; 1440; 10080; 43200";
extern bool       TimeFrame2bool = true;
extern int        TimeFrame2     = 240;
extern bool       TimeFrame3bool = true;
//-----
extern int        bands_shift = 0;
extern int        applied_price = 0;
//-----
double s1[];
double s2[];
double s3[];
//+------------------------------------------------------------------+
int init()
 {
   SetIndexBuffer(0, s1);
   SetIndexBuffer(1, s2);
   SetIndexBuffer(2, s3);
   //-----
   SetIndexStyle(0, DRAW_LINE);
   SetIndexStyle(1, DRAW_LINE);
   SetIndexStyle(2, DRAW_LINE);
   //-----
   SetIndexLabel(0, "s1");
   SetIndexLabel(1, "s2");
   SetIndexLabel(2, "s3");
   return(0);
 }
//+------------------------------------------------------------------+ 
int start()
 {
   int limit, x;
   double bandup_1, banddn_1, bandup_2, banddn_2;
   
   int counted_bars = IndicatorCounted();
   if(counted_bars < 0) return(-1);
   if(counted_bars > 0) counted_bars--;
   limit = Bars - counted_bars;
   //-----
   for(x = 0; x < limit; x++)
    {
      bandup_1 = iBands(NULL,0,20,1,bands_shift,applied_price,MODE_UPPER,x);
      banddn_1 = iBands(NULL,0,20,1,bands_shift,applied_price,MODE_LOWER,x);
      
      if( iClose(NULL,0,x) <= bandup_1 && iClose(NULL,0,x) >= banddn_1 )  { s1[x] = 1; }
    }
   //-----   
   if(TimeFrame2bool)
    {
      for(x = 0; x < limit; x++)
       {
         bandup_1 = iBands(NULL,TimeFrame2,20,1,bands_shift,applied_price,MODE_UPPER,iBarShift(NULL, TimeFrame2, Time[x], false));
         banddn_1 = iBands(NULL,TimeFrame2,20,1,bands_shift,applied_price,MODE_LOWER,iBarShift(NULL, TimeFrame2, Time[x], false));
       
         if( iClose(NULL,TimeFrame2,x) <= bandup_1 && iClose(NULL,TimeFrame2,x) >= banddn_1 )  { s2[x] = 1; }
       }
    } 
   //-----
   if(TimeFrame3bool)
    {
      for(x = 0; x < limit; x++)
       {
         if( s1[x] == 1 && s2[x] == 1 )  { s3[x] = 1; }
       }
    } 
   //-----
   string t = Period() + " = Gold";
   if(TimeFrame2bool) t = t + "\n" + TimeFrame2 + " = Green";
   if(TimeFrame3bool) t = t + "\n" + " = Blue";
   Comment(t);
   //=======================================================
   return(0);
 }
//+--------------------------------------------------------------------------+

int deinit()
 {
   Comment("");
 }
 

Below things I see:

As per your code, single for loop will suffice..It should looks something like this:

for(x = 0; x < limit; x++)
    {
      bandup_1 = iBands(NULL,0,20,1,bands_shift,applied_price,MODE_UPPER,x);
      banddn_1 = iBands(NULL,0,20,1,bands_shift,applied_price,MODE_LOWER,x);
      
      if( iClose(NULL,0,x) <= bandup_1 && iClose(NULL,0,x) >= banddn_1 )  { s1[x] = 1; }
      
      if(TimeFrame2bool)
      {
         bandup_1 = iBands(NULL,TimeFrame2,20,1,bands_shift,applied_price,MODE_UPPER,iBarShift(NULL, TimeFrame2, Time[x], false));
         banddn_1 = iBands(NULL,TimeFrame2,20,1,bands_shift,applied_price,MODE_LOWER,iBarShift(NULL, TimeFrame2, Time[x], false));
       
         if( iClose(NULL,TimeFrame2,x) <= bandup_1 && iClose(NULL,TimeFrame2,x) >= banddn_1 )  { s2[x] = 1; }
       }

      if(TimeFrame3bool)
      {
          if( s1[x] == 1 && s2[x] == 1 )  { s3[x] = 1; }
      } 
   }

In below

iClose(NULL,TimeFrame2,x)

change x to iBarShift, since its on different timeframe. Also add debug statements e.g. print in your code, to check if its returning correct value. Just for testing, set limit to small amount, say 20.

 
dineshydv:

Below things I see:

As per your code, single for loop will suffice..It should looks something like this:

In below

change x to iBarShift, since its on different timeframe. Also add debug statements e.g. print in your code, to check if its returning correct value. Just for testing, set limit to small amount, say 20.



Get it!

 

Thanks a lot! 

  
bandup_1 = iBands(NULL,TimeFrame2,20,1,bands_shift,applied_price,MODE_UPPER,iBarShift(NULL, TimeFrame2, Time[x], false));
banddn_1 = iBands(NULL,TimeFrame2,20,1,bands_shift,applied_price,MODE_LOWER,iBarShift(NULL, TimeFrame2, Time[x], false));
if( iClose(NULL,TimeFrame2,x) <= bandup_1 && iClose(NULL,TimeFrame2,x)
If you stop writeing unreadable code you would see the problem.
double GetBand(int eTF, int iTF, int eMODE){         // Hide common, non changing, external values.
   return( iBands(NULL,eTF,20,1,bands_shift,applied_price,eMODE,iTF) );
}
////////////
int  iBar = x;  // Use readable names. The chart's bar index. 'x' says nothing and in mathematics a double.
int  iTF2 = iBarShift(NULL, TimeFrame2, Time[iBar]);
double bandup_1 = GetBand(TimeFrame2, iTF2, MODE_UPPER),
       banddn_1 = GetBand(TimeFrame2, iTF2, MODE_LOWER),
       closeTF2 = iClose(NULL, TimeFrame2, iBar);   // <--- TimeFrame2 and iBAR are NOT compatible
                                                    //      TimeFrame2 and iTF2 ARE compatible
if (closeTF2 <= bandup_1 && closeTF2 ...
New comment
Reason: