Why is this simple program not working?

[thecerealslayer]

Hey i'm trying to build a simple program that comments Price Over when price is above the MA and Price Lower when below, but all the program is doing is saying "Price Lower" , can you explain why such a simple program is not working properly? In my code below you can see i commented out some code thats because it was having the same effect and also still only commenting "Price Lower".

void OnTick()
  {
  
  //create an array for the ma price
  double myMovingAverageArray1[];
  
  //define the properties of the moving average1
  int movingAverageDefinition1 = iMA(_Symbol,_Period,14,0,MODE_SMA,PRICE_CLOSE);
  
  //sort the array from newest to oldest
  ArraySetAsSeries(myMovingAverageArray1,true);
  
  
  //define the Moving Average 14
  CopyBuffer(movingAverageDefinition1,0,0,3,myMovingAverageArray1);
  

   //define the closing price
   double Close = PRICE_CLOSE;
  
  //if price is above ma its a buy signal
 
 
  
 if (Close > myMovingAverageArray1[0])
   {
   Comment("Price Over ");
   }else Comment("Price Lower");
 
 /* 
 if (Close < myMovingAverageArray1[0])
   {
   Comment("Price Lower");
   };
 */
  }

[Alain Verleyen]

Forum on trading, automated trading systems and testing trading strategies

How to debug with drawing on chart?

Alain Verleyen, 2023.07.31 01:04

Sure it's possible.

  //define the Macd EA
  int MacdDefinition = iMACD(_Symbol,_Period,6,15,9,PRICE_CLOSE);

Should not be in OnTick() but in OnInit().

  //defined Macd and store values, one buffer, current candle, 3 candles, store result
  CopyBuffer(MacdDefinition,0,0,3,myMacdPriceArray);
  CopyBuffer(MacdDefinition,1,0,3,myMacdSignalPriceArray);

CopyBuffer() is function which can fail and return a value. Always add error checking if you want some reliability.

 //get the value of the macd and signal current candle
  double MacdValue=(myMacdPriceArray[0]);
  double MacdSignalValue = myMacdSignalPriceArray[0];

Is it worth to answer you when you don't "listen" and are continuing to make the same mistakes ?

[thecerealslayer]

Alain Verleyen #:

Is it worth to answer you when you don't "listen" and are continuing to make the same mistakes ?

sorry man, i guess i forgot. I am following this guys tutorials so i didn't think twice about it. https://www.youtube.com/watch?v=AUEIEzCDZvw 

Also I tried listening to you on the previous thread and fixed things i thought correctly, but you never got back to me regarding if the new code was correct or not. After reading your note again i realized that my MacdDefinition was in a global scope not the init, so i moved it there and tried again still it didn't work to make any arrows. 

Regarding the highlight of CURRENT CANDLE and [0] i do not understand why you highlighted it as isn't [0] how you would get the current candle? From all the tutorials i have see it sounds like [0] would be correct.

Regarding the other thread you commented on, this was my attempt at catching an error, i don't know if it's correct: 

  //defined Macd and store values, one buffer, current candle, 3 candles, store result
  CopyBuffer(MacdDefinition,0,0,3,myMacdPriceArray);
  CopyBuffer(MacdDefinition,1,0,3,myMacdSignalPriceArray);
  
  //catch errors with copybuffer
  if (CopyBuffer(MacdDefinition,0,0,3,myMacdPriceArray)==-1){
   Print("Error copying Macd data to array.",GetLastError() );
   return;
  }
   if (CopyBuffer(MacdDefinition,1,0,3,myMacdSignalPriceArray)==-1){
   Print("Error copying Macd data to array.",GetLastError() );
   return;
  }

This is also the macd code now and still no arrow is being created on the chart: 

//MACD CODE ==================================================================
  
  //define the Macd (Moved to init)
  //int MacdDefinition = iMACD(_Symbol,_Period,6,15,9,PRICE_CLOSE);
  
  
  //create an array for several prices
  double myMacdPriceArray[];
  double myMacdSignalPriceArray[];
  //set to series to move data down on new tick
  ArraySetAsSeries(myMacdPriceArray,true);
  ArraySetAsSeries(myMacdSignalPriceArray,true);
  
  
  //defined Macd and store values, one buffer, current candle, 3 candles, store result
  CopyBuffer(MacdDefinition,0,0,3,myMacdPriceArray);
  CopyBuffer(MacdDefinition,1,0,3,myMacdSignalPriceArray);
  
  //catch errors with copybuffer
  if (CopyBuffer(MacdDefinition,0,0,3,myMacdPriceArray)==-1){
   Print("Error copying Macd data to array.",GetLastError() );
   return;
  }
   if (CopyBuffer(MacdDefinition,1,0,3,myMacdSignalPriceArray)==-1){
   Print("Error copying Macd data to array.",GetLastError() );
   return;
  }
  
  //get the value of the macd and signal current candle (looks like pointless code so i muted it)
  //double MacdValue=(myMacdPriceArray[0]);
  //double MacdSignalValue = myMacdSignalPriceArray[0];
  
  
  //buy signal for Macd (if lower than 0 and cross over signal)
  if (myMacdPriceArray[0]<0 && myMacdPriceArray[0] > myMacdSignalPriceArray[0] && myMacdPriceArray[1] < myMacdSignalPriceArray[1]){
   MacdBuyArrowID++; //(Arrow ID i just an int counter to allow multiple arrows.
   //set macd buy on if conditions are met
   MacdBuySignal=true;
   ObjectCreate(0,IntegerToString(MacdBuyArrowID), OBJ_ARROW_BUY, 0, TimeCurrent(),PRICE_CLOSE);
   Print("buy conditions are met for Macd on ",InpIdentifier );
   };
  

CURRENT FORUM COMMENT

Seeing that you wanted the init to hold the definition of the macd in the previous post i now moved the moving average definition to the init and also added an error catch to this aswell, The program is still not working with the changes. This is my new code: 

//+------------------------------------------------------------------+
//| Expert initialization function                                   |
//+------------------------------------------------------------------+
int movingAverageDefinition1 = 0;

int OnInit()
  {
  //define the properties of the moving average1
  movingAverageDefinition1 = iMA(_Symbol,_Period,14,0,MODE_SMA,PRICE_CLOSE);
  
  
   return(INIT_SUCCEEDED);
  }

//+------------------------------------------------------------------+
//| Expert deinitialization function                                 |
//+------------------------------------------------------------------+
void OnDeinit(const int reason)
  {

  }

//+------------------------------------------------------------------+
//| Expert tick function                                             |
//+------------------------------------------------------------------+
void OnTick()
  {
  
  //create an array for the ma price
  double myMovingAverageArray1[];
  
  
  //sort the array from newest to oldest
  ArraySetAsSeries(myMovingAverageArray1,true);
  
  
  //define the Moving Average 14
  CopyBuffer(movingAverageDefinition1,0,0,3,myMovingAverageArray1);
  
  if (CopyBuffer(movingAverageDefinition1,0,0,3,myMovingAverageArray1)==-1){
   Print("Error copying MA data to array.",GetLastError() );
   return;}

   //define the closing price
   double Close = PRICE_CLOSE;
  
  //if price is above ma its a buy signal
 
 
  
 if (Close > myMovingAverageArray1[0])
   {
   Comment("Price Over ");
   }else Comment("Price Lower");
 
 /* 
 if (Close < myMovingAverageArray1[0])
   {
   Comment("Price Lower");
   };
 */
  }

 I appreciate your patience

[thecerealslayer]

thecerealslayer #:

sorry man, i guess i forgot. I am following this guys tutorials so i didn't think twice about it. https://www.youtube.com/watch?v=AUEIEzCDZvw 

Also I tried listening to you on the previous thread and fixed things i thought correctly, but you never got back to me regarding if the new code was correct or not. After reading your note again i realized that my MacdDefinition was in a global scope not the init, so i moved it there and tried again still it didn't work to make any arrows. 

Regarding the highlight of CURRENT CANDLE and [0] i do not understand why you highlighted it as isn't [0] how you would get the current candle? From all the tutorials i have see it sounds like [0] would be correct.

Regarding the other thread you commented on, this was my attempt at catching an error, i don't know if it's correct: 

This is also the macd code now and still no arrow is being created on the chart: 

CURRENT FORUM COMMENT

Seeing that you wanted the init to hold the definition of the macd in the previous post i now moved the moving average definition to the init and also added an error catch to this aswell, The program is still not working with the changes. This is my new code: 

or is this a better way to look for errors for copybuffer? 

  if (CopyBuffer(movingAverageDefinition1,0,0,3,myMovingAverageArray1)==-1){
   Print("Error copying MA data to array.",GetLastError() );
   return;}else CopyBuffer(movingAverageDefinition1,0,0,3,myMovingAverageArray1);

[thecerealslayer]

Alain Verleyen #:

Is it worth to answer you when you don't "listen" and are continuing to make the same mistakes ?

Hey again, i just started to do a tutorial with someone else because aparently the other guy is half in the bag, but the new guy is doing what your talking about so i think i will stick to his tutorials from now on. 

here he is:

https://www.youtube.com/watch?v=h8lZCEpiFOI