Why is my indexing wrong?

dweems 2017.08.17 02:16

I'm just recently getting into coding MQL4 and I'm really confused with the results I'm seeing. This is perplexing since I'm a professional programmer with over 30 years experience, including nearly 10 years in C++. I'll illustrate my problem with two pieces of code. The first is the one that I think should work and the other does work but illogically. The idea is to identify when a trend switches by simply tracking two EMAs and drawing an arrow on the chart at the crossover point of the two. If the crossover indicates that the trend is moving down (i.e. the fast EMA is less than the slow EMA) then I draw a down arrow above that bar and if the crossover indicates that the trend is moving up (i.e. the fast EMA is greater than the slow EMO) draw an up arrow below the bar. The code that I think should be correct is off by one bar to the left, even though my indexing starts at 0, and the wrong arrow is drawn. The code that does work but illogically is where I decrement the index of the arrays for the arrows by one and switch the arrays for the conditions (i.e. add the price to the long array of arrows when I should be adding it to the short array and vice versa). Logically the indexing should fail because it provides the possibility that an array index could be -1. But I've seen it work on the first bar. This is very confusing. Please help me understand what I'm doing wrong.

================================================================

Code that should work but doesn't:

================================================================

#property strict
#property indicator_chart_window
#property indicator_buffers 2
#property indicator_type1 DRAW_ARROW
#property indicator_color1 Blue
#property indicator_type2 DRAW_ARROW
#property indicator_color2 Red

//---- input parameters

extern int percentArrowOffset = 5; // Percent of price range where arrows are displayed.
extern int emaPeriodFast = 4; // Exponential moving average period.
extern int emaPeriodSlow = 10; // Exponential moving average period.

//---- global buffers

double buyPivotPointsBuffer[];
double sellPivotPointsBuffer[];

//--- globals constants and variables

enum CurrentTrendType { kUnknown, kBuy, kSell};
CurrentTrendType currentTrend;

//+------------------------------------------------------------------+
//| Custom indicator initialization function                         |
//+------------------------------------------------------------------+

int OnInit()
{
    SetIndexBuffer(0, buyPivotPointsBuffer);
    SetIndexBuffer(1, sellPivotPointsBuffer);

    SetIndexStyle(0, DRAW_ARROW);
    SetIndexArrow(0, 217); // up arrow
    SetIndexLabel(0, "Buy Trend");

    SetIndexStyle(1, DRAW_ARROW);
    SetIndexArrow(1, 218); // down arrow
    SetIndexLabel(1, "Sell Trend");

    currentTrend = kUnknown;

    return (INIT_SUCCEEDED);
}

//+------------------------------------------------------------------+
//| Custom indicator iteration function                              |
//+------------------------------------------------------------------+

int OnCalculate(const int ratesTotal,
                const int prevCalculated,
                const datetime &time[],
                const double &open[],
                const double &high[],
                const double &low[],
                const double &close[],
                const long &tickVolume[],
                const long &volume[],
                const int &spread[])
{
    int numNewBars = ratesTotal - prevCalculated;

    if (numNewBars == 0)
        return (ratesTotal);

    double maxPrice = WindowPriceMax();
    double minPrice = WindowPriceMin();
    double priceRange = maxPrice - minPrice;
    double arrowPriceOffset = priceRange * (percentArrowOffset / 100.0);

    int lastIndex = ratesTotal - emaPeriodSlow - 1;
    for (int index = 0; index <= lastIndex; index++)
    {
        double emaFast = iMA(NULL, 0, emaPeriodFast, 0, MODE_EMA, PRICE_CLOSE, index);
        double emaSlow = iMA(NULL, 0, emaPeriodSlow, 0, MODE_EMA, PRICE_CLOSE, index);

        if (currentTrend == kBuy || currentTrend == kUnknown)
        {
            if (emaFast < emaSlow)
            {
                currentTrend = kSell;
                sellPivotPointsBuffer[index] = high[index] + arrowPriceOffset;
            }
        }
        else if (currentTrend == kSell || currentTrend == kUnknown)
        {
            if (emaFast > emaSlow)
            {
                currentTrend = kBuy;
                buyPivotPointsBuffer[index] = low[index] - arrowPriceOffset;
            }
        }
    }

    return (ratesTotal);
}

================================================================

Code that shouldn't work but does:

================================================================

#property strict
#property indicator_chart_window
#property indicator_buffers 2
#property indicator_type1 DRAW_ARROW
#property indicator_color1 Blue
#property indicator_type2 DRAW_ARROW
#property indicator_color2 Red

//---- input parameters
extern int percentArrowOffset = 5; // Percent of price range where arrows are displayed.
extern int emaPeriodFast = 4; // Exponential moving average period.
extern int emaPeriodSlow = 10; // Exponential moving average period.

//---- global buffers
double longPivotPointsBuffer[];
double shortPivotPointsBuffer[];

//--- globals constants and variables

enum CurrentTrendType { kUnknown, kLong, kShort };
CurrentTrendType currentTrend;

//+------------------------------------------------------------------+
//| Custom indicator initialization function                         |
//+------------------------------------------------------------------+

int OnInit()
{
    SetIndexBuffer(0, longPivotPointsBuffer);
    SetIndexStyle(0, DRAW_ARROW);
    SetIndexArrow(0, 217); // up arrow
    SetIndexLabel(0, "Buy Trend");

    SetIndexBuffer(1, shortPivotPointsBuffer);
    SetIndexStyle(1, DRAW_ARROW);
    SetIndexArrow(1, 218); // down arrow
    SetIndexLabel(1, "Sell Trend");

    currentTrend = kUnknown;

    return (INIT_SUCCEEDED);
}

//+------------------------------------------------------------------+
//| Custom indicator iteration function                              |
//+------------------------------------------------------------------+

int OnCalculate(const int ratesTotal,
                const int prevCalculated,
                const datetime &time[],
                const double &open[],
                const double &high[],
                const double &low[],
                const double &close[],
                const long &tickVolume[],
                const long &volume[],
                const int &spread[])
{
    int numNewBars = ratesTotal - prevCalculated;

    if (numNewBars == 0)
        return (ratesTotal);

    double maxPrice = WindowPriceMax();
    double minPrice = WindowPriceMin();
    double priceRange = maxPrice - minPrice;
    double arrowPriceOffset = priceRange * (percentArrowOffset / 100.0);

    int lastIndex = ratesTotal - emaPeriodSlow - 1;

    for (int index = 0; index <= lastIndex; index++)
    {
        double emaFast = iMA(NULL, 0, emaPeriodFast, 0, MODE_EMA, PRICE_CLOSE, index);
        double emaSlow = iMA(NULL, 0, emaPeriodSlow, 0, MODE_EMA, PRICE_CLOSE, index);

        if (currentTrend == kLong || currentTrend == kUnknown)
        {
            if (emaFast < emaSlow)
            {
                currentTrend = kShort;
                longPivotPointsBuffer[index-1] = low[index-1] - arrowPriceOffset;
            }
        }

        else if (currentTrend == kShort || currentTrend == kUnknown)
        {
            if (emaFast > emaSlow)
            {
                currentTrend = kLong;
                shortPivotPointsBuffer[index-1] = high[index-1] + arrowPriceOffset;
            }
        }
    }

    return (ratesTotal);
}

Keith Watford 2017.08.17 05:03 #1

Please use the SRC button when posting code. I have done it for you this time, but please remember in future.

    int lastIndex = ratesTotal - emaPeriodSlow - 1;
    for (int index = 0; index <= lastIndex; index++)
    {
        double emaFast = iMA(NULL, 0, emaPeriodFast, 0, MODE_EMA, PRICE_CLOSE, index);
        double emaSlow = iMA(NULL, 0, emaPeriodSlow, 0, MODE_EMA, PRICE_CLOSE, index);

        if (currentTrend == kBuy || currentTrend == kUnknown)
        {
            if (emaFast < emaSlow)
            {
                currentTrend = kSell;
                sellPivotPointsBuffer[index] = high[index] + arrowPriceOffset;
            }
        }
        else if (currentTrend == kSell || currentTrend == kUnknown)
        {
            if (emaFast > emaSlow)
            {
                currentTrend = kBuy;
                buyPivotPointsBuffer[index] = low[index] - arrowPriceOffset;
            }
        }
    }

You are counting up instead of down, this means that the variable currentTrend is set by the calculation from the bar [lastIndex]. This makes no sense and will obviously cause miscalculations.

Why are you recalculating so many bars anyway?

You are calculating bar[0] when it first opens and maybe giving the arrow buffer a value. By the time that the bar has closed, conditions for the arrow may no longer be true but you do nothing to remove it.

Engulfing indicator Need help please with My FX strategy -

dweems 2017.08.17 18:39 #2

I realized last night in my sleep that I was redundantly recalculating the data needlessly. And I completely acknowledge that I would have to reverse the loop when it came time to converting this to a function that I will use in an EA to get the correct trend direction. So I made the changes and, sure enough, it works. Thanks for that. I'm really confused that my illogical solution worked at all. I started this way to test the functionality because I see example code in the reference that starts at zero. I should have looked deeper. The good news is I have a better understanding of how the data is coming into OnCalculate() and when. Thanks again. (And next time I will use the SRC button.)

Error 4806 CopyBuffer() With Any rookie question, so trouble with OrderClose()

William Roeder 2017.08.17 20:43 #3

dweems: I was redundantly recalculating the data needlessly.

See How to do your lookbacks correctly.
Your currentTrend variable a value from the previous iteration, which means you can't process bar zero more than once, ever. Put it in a buffer so you can restore it, or only process bar zero once, or save and restore them when processing bar zero.

Weird kinks in my Need some help to MQL4 buffer trick, I

GrumpyDuckMan 2017.08.20 02:57 #4

Hello all,

I was playing around with OP's code and this is what I ended up with. To me it seems to work the way he wanted it to.

I only really changed the ma_shift to -1. looks to work.

#property strict
#property indicator_chart_window
#property indicator_buffers 2
#property indicator_type1 DRAW_ARROW
#property indicator_color1 Blue
#property indicator_type2 DRAW_ARROW
#property indicator_color2 Red

//---- input parameters
input int TIMEFRAME=0;
input int percentArrowOffset=0;
input int emaPeriodFast=9;
input int emaPeriodSlow=26;

//---- global buffers
double longPivotPointsBuffer[];//Buy
double shortPivotPointsBuffer[];//Sell
int Chart_ID;
string charts;
long chartid;
//--- globals constants and variables

enum CurrentTrendType {kUnknown,kLong,kShort};CurrentTrendType

currentTrend;
//+------------------------------------------------------------------+
//| Custom indicator initialization function                         |
//+------------------------------------------------------------------+

int OnInit()
  {
   bool TimeFunction=ChartSetSymbolPeriod(chartid,charts,TIMEFRAME);
   ChartSetInteger(Chart_ID,CHART_VISIBLE_BARS,1);
   ChartSetInteger(Chart_ID,CHART_MODE,CHART_CANDLES);
   ChartSetInteger(Chart_ID,CHART_SHOW_GRID,false);
   ChartSetInteger(Chart_ID,CHART_AUTOSCROLL,true);
   ChartSetInteger(Chart_ID,CHART_SCALE,4);
   ChartSetInteger(Chart_ID,CHART_SHIFT,1);

   SetIndexBuffer(0,longPivotPointsBuffer);
   SetIndexStyle(0,DRAW_ARROW);
   SetIndexArrow(0,217);  // up arrow
   SetIndexLabel(0,"Buy Trend");

   SetIndexBuffer(1,shortPivotPointsBuffer);
   SetIndexStyle(1,DRAW_ARROW);
   SetIndexArrow(1,218);  // down arrow
   SetIndexLabel(1,"Sell Trend");

   currentTrend=kUnknown;

   return (INIT_SUCCEEDED);
  }
//+------------------------------------------------------------------+
//| Custom indicator iteration function                              |
//+------------------------------------------------------------------+

int OnCalculate(const int ratesTotal,
                const int prevCalculated,
                const datetime &time[],
                const double &open[],
                const double &high[],
                const double &low[],
                const double &close[],
                const long &tickVolume[],
                const long &volume[],
                const int &spread[])
  {
   int numNewBars=ratesTotal-prevCalculated;
   if(numNewBars==0)
      return (ratesTotal);

   double maxPrice = WindowPriceMax();
   double minPrice = WindowPriceMin();
   double priceRange=maxPrice-minPrice;
   double arrowPriceOffset=priceRange *(percentArrowOffset/100.0);
   int lastIndex=ratesTotal-emaPeriodSlow;

   for(int index=0; index<=lastIndex; index++)
     {

      double emaFast = iMA(NULL,TIMEFRAME,emaPeriodFast,-1,MODE_EMA,PRICE_CLOSE,index);
      double emaSlow = iMA(NULL,TIMEFRAME,emaPeriodSlow,-1,MODE_EMA,PRICE_CLOSE,index);

      if(currentTrend==kLong || currentTrend==kUnknown)
        {
         if(emaFast<emaSlow)
           {
            currentTrend=kShort;
            longPivotPointsBuffer[index]=Low[index]+arrowPriceOffset;
           }
        }
      else if(currentTrend==kShort || currentTrend==kUnknown)
        {
         if(emaFast>emaSlow)
           {
            currentTrend=kLong;
            shortPivotPointsBuffer[index]=High[index]-arrowPriceOffset;
           }
        }
     }
   return (ratesTotal);
  }
//+------------------------------------------------------------------+

how do i hide is there some "StringToEnum" In MQL5 there is

William Roeder 2017.08.20 11:52 #5

GrumpyDuckMan: I was playing around with OP's code and this is what I ended up with. To me it seems to work the way he wanted it to.

for(int index=0; index<=lastIndex; index++){
      double emaFast = iMA(NULL,TIMEFRAME,emaPeriodFast,-1,MODE_EMA,PRICE_CLOSE,index);
      double emaSlow = iMA(NULL,TIMEFRAME,emaPeriodSlow,-1,MODE_EMA,PRICE_CLOSE,index);

      if(currentTrend==kLong || currentTrend==kUnknown){
         if(emaFast<emaSlow) currentTrend=kShort;

You are using newer values to define (the reverse) of the trend in the past. Bogus.
And when you reverse it, it still has problem #3 Item 2

GrumpyDuckMan 2017.08.20 12:10 #6

whroeder1:

You are using newer values to define (the reverse) of the trend in the past. Bogus.
And when you reverse it, it still has problem #3 Item 2

Hello again,

I learn by mistakes.

Thank you

New comment