EA Template

Admin
Please review the code
Thanks
F

Another Free EA:


Linkback: https://www.forex.zone/mt4-mt5-eas/7/ea-template/611/

I reviewed the code and here is what I would change. If you decide to make changes, I can review the code later in more detail.


The timer should be updated. I think I remember you mentioning this on the forum in another place. I would be happy to give you better timer code. The problem with your code is that if someone wants to trade from 17 at night to 7 in the morning instead of 7 to 17 then your code wont work.

In line 109, there is a bracket that is unneeded. It has a matching end bracket so it doesn't cause an error, but this set of brackets is unneeded and doesn't accomplish anything.

In the open_orders(), there are if statements following each other without brackets. I think this is bad coding practice, even if it may work.

For buyfunc() and sellfunc():

• You have a bool variable set for the return value of OrderSend(). The OrderSend() returns the ticket number if successful and not a bool value.
• It's better to use normalized values for the entry prices. For example, 'NormalizeDouble(Ask,Digits)" instead of just 'Ask'. This will prevent errors during back testing.
• The function returns true even if the order can fail. It's better to at least do a simple check for a valid ticket # and then return true. If it failed, you can retry to place the order again too.

For movetobreakeven() and trailstop():

• There is the same missing bracket issue that I mentioned above.
• These functions will only run if the time is within allowable hours and the volume signal is good. The problem is that I think you would want the EA to still manage existing trades even if the time and volume are no good.
• Instead of cycling through the order pool twice in the same function, you can use a single cycle and have a simple check for buy or sell orders. This will save some repeated code and increase speed.

Here is a better timer code. I have not used it in a long time, but I think it will work. If you test and see that it doesn't, just let me know.


Use this for the user adjustable inputs. It uses hours and minutes. Keep the same format as the sample. So don't add a '.' instead of a ':' or a comma instead of a hyphen.
extern bool   Use_Time_Restrictions = false;
extern string Times_Allowed = "08:00-16:00";

Here is the main function. If it returns true, then you can trade. If false, then no new trades.
bool TimeFilter()
 {
  if (!Use_Time_Restrictions)return(true);
  if (Times_Allowed=="" || Times_Allowed==" ")return(true);
 
  int pos = StringFind(Times_Allowed,"-",0);
  datetime top = StrToTime(StringTrimLeft(StringTrimRight(StringSubstr(Times_Allowed,0,pos))));
  datetime tcl = StrToTime(StringTrimLeft(StringTrimRight(StringSubstr(Times_Allowed,pos+1))));
  if(top<tcl)
   {
    if(TimeCurrent()>=top&&TimeCurrent()<tcl)return(true);
    else return(false);
   }
  else if (top>tcl)
   {
    if(TimeCurrent()>=top||TimeCurrent()<tcl)return(true);
    else return(false);         
   }
  
  return(true);
 }

Here are some more suggestions in addition to any previous recommendations.

• On line 96-97, you can leave out the NULL and just input the object name. This function should either be used with a chart number or not.

• You can also do an easy check to see if the object exists before trying to delete it. This will prevent errors in your logging sometimes. For example, if the user manually deletes an object or if another program deletes an object before your EA does.

if (ObjectFind("Time Box Inside")>=0)ObjectDelete("Time Box Inside");

• In line 109, instead of normalizing the lot size like this, it would be best if you based it on the specific broker rules. For example, some brokers may not allow a lot step of 0.01 but might require 0.05 or 0.10. Also, the broker's min lot size should be checked too. This also is not a big deal, but will prevent missed orders and errors in the logging later. I can give you some sample code if you want.

• For the StopLoss and TakeProfit inputs, it would be best to only set TP and SL values if the user has set something >0. You have it checking if the input !=0. So if someone enters a negative number, your code will try to set a TP and SL. I have seen people do this before. So any negative numbers could be treated the same as if the user has input a 0.

• On line 137, you might want to split up the spread and new bar checks. If it's a new bar, but the spread is no good then the new bar function still updates the variable and the EA will have to wait for a new bar again. What can happen is a couple ticks later the spread may come into allowable levels, but it is no longer a new bar. An easy way to fix this is just to reverse the checks. So put the spread check first and then the new bar check to the right side. If the spread is no good then, the NewBar() function would not run then.

• On line 173 there is an empty TimeFilter() check.

On NewBar and Spread:


If I change it around, and the spread comes back down to a desired level after say 50 ticks, will the NewBar not be false then anyway?

No, because the first check will be made and if it's false then the rest of the statement will be exited. Which means the NewBar() wouldn't have been reached in the code so the variable would not have been updated. Unless Metaquotes has changed this, but I am sure if a check is false then the program will stop the rest of the statement. 

Personally, I would separate the checks and make it so NewBar() isn't even gotten too unless the spread is good on the previous line.

Line 173 - I copied and pasted the TimeFilter code exactly as I received it, but I will take it out anyway. BTW, well done, you said you haven't used it in while, and it still works 

Unless you uploaded the wrong file, there was just an empty check. In other words, it checked the TimeFilter(), but didn't do anything about it. It was however used previously in the code so I think maybe you just forgot to delete it.

I've sorted it out, thanks anyway.

How to implement MODE_LOTSTEP still gets to me a bit, but I figured brokers have a lot steps of either 0.01, 0.05 0.1 or 1,
either way if the lot is 0.04, NormalizeDouble will take it to 0.01, if it's 0.06, it will go to 0.1?
So I am just basing the lot calculation on MarketInfo(Symbol()or NULL,MODE_MIN_LOT);
Fortunately, the broker I am testing the EA on has 0.1 LotStep and MinLot  

Thanks, I will pop it into a script and see how to implement it