Array out of range , Help please

New comment
 

Hello, first of all I apologize if I'm posting this in the wrong place.


In my expert, I prepare a code like this to find the last 2 highs and lows of the zigzag indicator.

When I check it as script, the code works fine and gives correct peaks and troughs.

but when i run experti in strategy tester i get out of array error...


      tepe1=zigzagtepe[i];


exactly on this line.


When I put this line in the for loop just before the break command, I don't get this error, but this time it doesn't determine the peaks and bottoms correctly.


can you help me with this.


 double zigzagtepe[],zigzagdip[];
 ArraySetAsSeries(zigzagtepe,true);
 ArraySetAsSeries(zigzagdip,true);
 double tepe=iCustom(NULL,zigzagtime,"Examples/ZigzagColor",12,5,3);
 double dip=iCustom(NULL,zigzagtime,"Examples/ZigzagColor",12,5,3);
 CopyBuffer(tepe,0,0,bar,zigzagtepe);
 CopyBuffer(dip,1,0,bar,zigzagdip);
 int bar=500;
 int i;     
 for(i=0;i<bar;i++)
  {
   if(zigzagtepe[i]>0)
   break;
  }
    tepe1=zigzagtepe[i];
    zigzagtepemum1=i;
 for(i=zigzagtepemum1+1;i<bar;i++)
  {
   if(zigzagtepe[i]>0)
   break;
  }
    tepe2=zigzagtepe[i];

 for(i=0;i<bar;i++)
  {
   if(zigzagdip[i]>0)
   break;
  }
    dip1=zigzagdip[i];
    zigzagdipmum1=i;
 for(i=zigzagdipmum1+1;i<bar;i++)
  {
   if(zigzagdip[i]>0)
   break;
  }
    dip2=zigzagdip[i];
 

Hi,

Try to define int bar=500 before copybuffer and not after.

 
  1. Style your code and correct the indentation. This is hard to read.

  2. Never, ever reuse loop indices. This is the cause of the error. 

  3. When all zigzagtepe[] values are equal zero, your loop never hits break. In that situation variable "i" is incremented and after the loop finishes, it's value is equal to variable "bar".
    After the loop finishes, you try to read zigzagtepe[bar] which does not exist.

  4. You should do something like this:


var notZeroIndex = -1;
for(int i=0; i<bar; i++)
{
   if(zigzagtepe[i]>0)
   {
        notZeroIndex = i;
        break;
  }
}
if(notZeroIndex >= 0)
{
    tepe1 = zigzagtepe[notZeroIndex];
    zigzagtepemum1 = notZeroIndex ;
}
else
{
  // do something else when all values in the array are zero
}


Above code is not tested nor compiled, but you get the idea.

And remember to never reuse loop indices. It is bad practice that leads to bugs.

 
Charles, Marie Tilly #:

Hi,

Try to define int bar=500 before copybuffer and not after.

I tried that way but the problem is the same..


but i solved the problem in another way.

that I defined at the beginning;

double zigzagtepe[],zigzagdip[]; variables

double zigzagtepe[500],zigzagdip[500];

I changed it to... and I'm not getting any errors...


I guess when expert first ran it was giving an error because the size of the defined arrays was not certain... now there is no problem.

 
Drazen Penic #:
  1. Style your code and correct the indentation. This is hard to read.

  2. Never, ever reuse loop indices. This is the cause of the error. 

  3. When all zigzagtepe[] values are equal zero, your loop never hits break. In that situation variable "i" is incremented and after the loop finishes, it's value is equal to variable "bar".
    After the loop finishes, you try to read zigzagtepe[bar] which does not exist.

  4. You should do something like this:



Above code is not tested nor compiled, but you get the idea.

And remember to never reuse loop indices. It is bad practice that leads to bugs.

yes you enlighten me logically it seems impossible for that loop to completely end with 0, but it would be more accurate to guarantee the job as you did.


but I don't fully understand the case of not reusing indexes. Can you open a little more?

 
Ümit UYSAL #:

I tried that way but the problem is the same..


but i solved the problem in another way.

that I defined at the beginning;

double zigzagtepe[],zigzagdip[]; variables

double zigzagtepe[500],zigzagdip[500];

I changed it to... and I'm not getting any errors...


I guess when expert first ran it was giving an error because the size of the defined arrays was not certain... now there is no problem.

no need to define them twice it serves no purpose and can be confusing.... just do the highlighted version.

  
 double tepe=iCustom(NULL,zigzagtime,"Examples/ZigzagColor",12,5,3);
 double dip=iCustom(NULL,zigzagtime,"Examples/ZigzagColor",12,5,3);
 CopyBuffer(tepe,0,0,bar,zigzagtepe);
 CopyBuffer(dip,1,0,bar,zigzagdip);
 int bar=500;
 int i;     
 for(i=0;i<bar;i++)
  1. Does this even compile? Defining bar after using it in CopyBuffer.
  2. You assume that CopyBuffer returns bar values. It may not. Loop over what it has actually returned.

  3. Perhaps you should read the manual, especially the examples.
       How To Ask Questions The Smart Way. (2004)
          How To Interpret Answers.
             RTFM and STFW: How To Tell You've Seriously Screwed Up.

    They all (including iCustom) return a handle (an int). You get that in OnInit. In OnTick/OnCalculate (after the indicator has updated its buffers), you use the handle, shift and count to get the data.
              Technical Indicators - Reference on algorithmic/automated trading language for MetaTrader 5
              Timeseries and Indicators Access / CopyBuffer - Reference on algorithmic/automated trading language for MetaTrader 5
              How to start with MQL5 - General - MQL5 programming forum - Page 3 #22 (2020)
              How to start with MQL5 - MetaTrader 5 - General - MQL5 programming forum - Page 7 #61 (2020)
              MQL5 for Newbies: Guide to Using Technical Indicators in Expert Advisors - MQL5 Articles (2010)
              How to call indicators in MQL5 - MQL5 Articles (2010)

 
Ümit UYSAL #:

yes you enlighten me logically it seems impossible for that loop to completely end with 0, but it would be more accurate to guarantee the job as you did.


but I don't fully understand the case of not reusing indexes. Can you open a little more?


You are declaring "int i" before loop, and then you use that same variable three times as a loop index and also between loops.

Don't do that. Using single variable for multiple purposes very often leads to bugs (as in your example).

For for loops always define variable in place. Loop index variable should only be loop index and nothing else:

for(int i = 0; i < something; i++)
{
// some code
}

for(int i = somethingelse; i >= 0; i--)
{
// some other code
}


Try to keep variable scope minimal.

It increases code readability, it helps debugging, it helps reduce number of bugs, it helps code maintenance.
 

New comment
Reason: