for loop to check OsMA falling for 'n' no of bars ...

New comment
 

UPDATE: Issues is sorted out

Hi

Please help me, what is wrong in my code and it loops only once and not for no of times passed in with pLookBack ...

//+----------------------------------------------------------------------------------------------------------+
//| METHOD:       Is_OsMA_Falling()
//| APPLICATION:  method to check if MACD OsMA Falling in specified bars on specified Time Frame
//+----------------------------------------------------------------------------------------------------------+
bool Cs_MACD::Is_OsMA_Falling(const ENUM_TIMEFRAMES pTimeFrame,const int pLookBack)
  {
    int barCount = pLookBack + 1;    // added Index[0] to counter
  //--- Minimum pLookBack is 3 bars
    if(barCount < 3)
      barCount = 3;
  //--- Declare local array structure to hold values
    mql_VWMACD aMACD[];
  //--- Refresh MACD Data Buffers depending on input Time Frame
    if(pTimeFrame == m_TF_Main)
      MACD_Main.Get_Data(barCount,aMACD);
    else if(pTimeFrame == m_TF_Lower)
      MACD_Lower.Get_Data(barCount,aMACD);
    else if(pTimeFrame == m_TF_Trend)
      MACD_Trend.Get_Data(barCount,aMACD);
    else
      {
        Print(__FUNCTION__,": Function not yet defined for TimeFrame "+(string)pTimeFrame);
        return(false);
      }
  //--- Check if OsMA is 'falling' for specified no of bars
    int startBar = (barCount - 1);              // Index[0] excluded
    int count = 0;                              // initiliaze counter
    for(int i = 1; i < startBar && !IsStopped(); i++)
      {
        if(aMACD[i].osma > aMACD[i-1].osma)
          {
            count++;
            PrintFormat("%s: OsMA index [%d] value %.5f",(string)__FUNCTION__,(int)i,aMACD[i].osma);
          }
        if(count != startBar)
          return(false);
      }
    return(true);     // if for..loop completes with all if conditions as true
  } // END Of method to check Is_OsMA_Falling()
 
Anil Varma -:

Hi

Please help me, what is wrong in my code and it loops only once and not for no of times passed in with pLookBack ...

Is it that hard to debug your code ?

Are you at least using the debugger to check it ?

 

You should explain in words, or by comments inside your code, what you are expecting this code to do.

Could be that aMACD is not yet filled, and you don't have property strict set - in this case you'll just get a silent failure.

 
Alain Verleyen:

Is it that hard to debug your code ?

Are you at least using the debugger to check it ?

Thanks Alain.

I never tried using debugger, will do so.

 
lippmaje:

You should explain in words, or by comments inside your code, what you are expecting this code to do.

Could be that aMACD is not yet filled, and you don't have property strict set - in this case you'll just get a silent failure.

well this checks one value then comes out of loop.

Anyway as suggested by Alain, I will try using debugger and see if can find out what is wrong.

Thanks a lot for your reply.

 
Anil Varma -:

well this checks one value then comes out of loop.

Anyway as suggested by Alain, I will try using debugger and see if can find out what is wrong.

Thanks a lot for your reply.

Yes, it may loop just once depending on the element numbers of itens of the array.

There is a logic error, which may cause that.


for(int i = 1; i < startBar && !IsStopped(); i++)
      {
        if(aMACD[i].osma > aMACD[i-1].osma)
          {
            count++;
            PrintFormat("%s: OsMA index [%d] value %.5f",(string)__FUNCTION__,(int)i,aMACD[i].osma);
          }
        if(count != startBar)
          return(false);
      }


Take a look at what is being told for the loop to do, and to exit:

if(count != startBar)


It will run once, since count is different of startBar!

count is initialized with zero

Startbar is another value, does not matter what value it is.. lets suppose it is = 10


On the first loop, if (count != startbar) than return.. It is done, end of loop. 

(if zero is different of 10, return)


They will almost 99% of the time (if not 100%) be different.. This logic is wrong

 
rrocchi:


YES, totally agree with you.

You have pinpointed the exact cause of my foolishness :) in coding. Sometime it happens when you work all day long.

Thanks a lot.

I have tried another logic, and request you to have a look if it make sense.

 //+--------------------------------------------------------------------------------------------------------+
  //| Check if the market is in RANGE or CONSOLIDATION
  //+--------------------------------------------------------------------------------------------------------+
    else if((MACD[1].osma < macd_Threshold) && (MACD[1].osma > -macd_Threshold))                // if Index[1] in range
      {
        for(int j = 1; j <= pRangeLookBack && !IsStopped(); j++)                                // loop to check pRangeLoockBack bars, if they also in Range
          {
            if((MACD[j].osma < macd_Threshold) && (MACD[j].osma > -macd_Threshold))
              continue;                                                                         // if true, get the next candle of for..loop
            else
              getTrend = TREND_UNKNOWN;                                                         // if false, return Trend Unknown
          }                                                                                     // if completes the whole loop with true values
        getTrend = RANGING;                                                                     // return Trend RANGING ...
      }
 
Anil Varma -:
You're welcome.
 
Anil Varma -:

YES, totally agree with you.

You have pinpointed the exact cause of my foolishness :) in coding. Sometime it happens when you work all day long.

Thanks a lot.

I have tried another logic, and request you to have a look if it make sense.

//+--------------------------------------------------------------------------------------------------------+
  //| Check if the market is in RANGE or CONSOLIDATION
  //+--------------------------------------------------------------------------------------------------------+
    else if((MACD[1].osma < macd_Threshold) && (MACD[1].osma > -macd_Threshold))                // if Index[1] in range
      {
        getTrend = RANGING; // Set as default, before the loop.

        for(int j = 1; j <= pRangeLookBack && !IsStopped(); j++)                                // loop to check pRangeLoockBack bars, if they also in Range
          {
            if((MACD[j].osma < macd_Threshold) && (MACD[j].osma > -macd_Threshold)) {
              continue; 
            }                                                                           // if true, get the next candle of for..loop
            else {
              getTrend = TREND_UNKNOWN;                                                // if false, return Trend Unknown
              break;                                             //You neeed to put a BREAK here, if what you want is to get out of the 'for loop' when it reaches here
            }                                                                                   
          }                                                                                     // if completes the whole loop with true values
      }


Set getTrend as RANGING before entering the loop, this is the final value for it IF the it completes the whole loop with TRUE values.

In case of the loop entering the ELSE situation, it will change getTrend to TREND_UNKNOW, and the next command will BREAK the loop.


This way, you will always have GetTrend = RANGING if the loops complete all the comparisions as TRUE.

otherwise as soon as the first comparison fails, GetTrend will be TREND_UNKNOWN, and loop will break.. 

You dont have to loop everything up to the end if you encounter a value on the way which is wrong.. just break and return.

 
rrocchi:


Set getTrend as RANGING before entering the loop, this is the final value for it IF the it completes the whole loop with TRUE values.

In case of the loop entering the ELSE situation, it will change getTrend to TREND_UNKNOW, and the next command will BREAK the loop.


This way, you will always have GetTrend = RANGING if the loops complete all the comparisions as TRUE.

otherwise as soon as the first comparison fails, GetTrend will be TREND_UNKNOWN, and loop will break.. 

You dont have to loop everything up to the end if you encounter a value on the way which is wrong.. just break and return.

Thanks rrocchi,

Yes got your concept very clearly and applied on my EA. It is working well.

 
Anil Varma -:

Thanks rrocchi,

Yes got your concept very clearly and applied on my EA. It is working well.

you are welcome

New comment