Reading indicator buffers with CopyBuffer in MQL5 without off-by-one bugs
CopyBuffer is the only supported way to read an indicator's values from an Expert Advisor in MQL5, and it is also one of the easiest places to introduce a silent, one-bar error. Nothing throws, nothing prints. The EA simply acts on a value that was true one bar ago, and the backtest still looks plausible enough that you go looking somewhere else. Here are the three ways it happens, and the code that avoids them.
The symptom
You have a moving-average crossover. The logic reads the last two values, compares them, and trades on the cross. In the tester the trades consistently appear one bar *after* the cross you can see on the chart. Or the EA reads a buffer for a stop level and gets last bar's value, so the stop sits slightly wrong. Or the first signals after a new bar forms are missing entirely. Different symptoms, same family of causes.
Cause one: two opposite indexing conventions
Inside OnCalculate, indicator buffers run oldest-first. buffer[0] is the oldest bar in the array; buffer[rates_total - 1] is the current bar. That is the opposite of MQL4, where buffers were timeseries.
CopyBuffer does not use that convention by default. It fills the destination array according to that array's own AS_SERIES flag:
- flag not set: element 0 is the oldest of the copied range, the last element is the newest;
ArraySetAsSeries(dest, true): element 0 is the newest.
So this is wrong:
double buf[];
CopyBuffer(ma_handle, 0, 0, 3, buf);
double latest = buf[0]; // no flag set: this is two bars ago, not nowbuf[0] is the oldest of the three. Without the flag, the current value is buf[2]. With the flag, it is buf[0]. Pick one convention and use it everywhere.
Cause two: the flag is set in the wrong place
ArraySetAsSeries must be called on the destination array, before CopyBuffer. It is a property of that array object, so a freshly declared local array needs it again. A pattern that works:
bool ReadMA(const int handle, const int shift, double &value)
{
double buf[];
ArraySetAsSeries(buf, true); // index 0 = most recent
const int copied = CopyBuffer(handle, 0, shift, 1, buf);
if(copied != 1)
{
PrintFormat("CopyBuffer returned %d, error %d", copied, GetLastError());
return false;
}
value = buf[0];
return true;
}Note the copied != 1 check. That is the next cause.
Cause three: partial copies and handles that are not ready
CopyBuffer returns the number of elements copied, or -1 on error. It is entirely normal for it to copy fewer elements than you asked for. Request five bars from a fresh handle and you may get two, because the indicator has not finished calculating. After a weekend gap, a symbol change or a history download you can get zero.
The destination dynamic array is resized to the number of elements actually copied. If you ignore the return value and read buf[4], you are indexing past the end of a shorter array — which raises a runtime error and kills the EA. When you request the last N bars you should require all N:
double ma[];
ArraySetAsSeries(ma, true);
const int copied = CopyBuffer(ma_handle, 0, 0, 3, ma);
if(copied < 3)
return; // history or indicator not ready; skip this tick
const double current = ma[0];
const double prev = ma[1];
const double prev2 = ma[2];Separately, iMA, iRSI, iCustom and friends return a handle immediately, before any data exists. BarsCalculated(handle) returns -1 on error and 0 while the indicator is still calculating. Gate on it:
bool IndicatorReady(const int handle)
{
const int calculated = BarsCalculated(handle);
if(calculated < 0)
{
PrintFormat("BarsCalculated failed, error %d", GetLastError());
return false;
}
return calculated > 0;
}Call that from OnTick and return early until it is true. Do not wrap it in a Sleep loop; that stalls the EA thread and delays every other tick you should be handling.
The same off-by-one inside OnCalculate
One more place it hides. When you write your own indicator, the restart index derived from prev_calculated is easy to get wrong, because prev_calculated counts bars and so sits one past the last valid index:
int OnCalculate(const int rates_total,
const int prev_calculated,
const datetime &time[],
const double &open[],
const double &high[],
const double &low[],
const double &close[],
const long &tick_volume[],
const long &volume[],
const int &spread[])
{
if(rates_total < 30)
return 0; // not enough history this call
const int start = (prev_calculated > 1) ? prev_calculated - 1 : 0;
for(int i = start; i < rates_total; i++)
ExtBuffer[i] = close[i]; // your calculation goes here
return rates_total; // must be rates_total, not -1
}Two traps there. prev_calculated - 1 deliberately recomputes the last bar, which you normally want because the newest bar is still forming. And the return value becomes the next call's prev_calculated, so it has to be rates_total.
A short checklist
- Choose one convention. I use
ArraySetAsSeries(dest, true), so index 0 is always the current bar. - Call it before every
CopyBufferon that array. - Compare the return value against the number of elements you asked for.
- Check
BarsCalculatedbefore the first read of a new handle. - Inside
OnCalculate, buffers are oldest-first and the last valid index isrates_total - 1.
Once fixed, your entry bars should shift by exactly one. If you want to see what correct timing looks like against a known result, [see a real backtest](https://mql.ranartech.com/r/EJHG0Xu8jDJh) — EURUSD H1 EA (+$151, 57% win, 92 trades) — and line the entry bars up against the indicator crosses on the chart.
Build & backtest your own EA free →