Forex Zone - Forex Forum

EA Template

Discussion started on MT4 / MT5 EAs

  • Sr. Member
  • Posts: 352
  • Points: 71
  • Likes Received: 150
  • Reputation: +11/-0
Admin
Please review the code
Thanks
F

Linkback: https://www.forex.zone/mt4-mt5-eas/7/ea-template/611/
#1 - December 19, 2018, 04:11:58 PM
Attachments:

Administrator
  • Hero Member
  • Posts: 4732
  • Points: 34629
  • Likes Received: 4838
  • Reputation: +220/-14
I got your newest PM. I will look into this ASAP.
#2 - December 19, 2018, 06:13:53 PM

Administrator
  • Hero Member
  • Posts: 4732
  • Points: 34629
  • Likes Received: 4838
  • Reputation: +220/-14
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.

#3 - December 19, 2018, 10:15:47 PM

  • Sr. Member
  • Posts: 352
  • Points: 71
  • Likes Received: 150
  • Reputation: +11/-0
Thanks
I thought all my close functions were outside the TradeTime :-[, thanks for pointing out my mistakes.

I would really appreciate a different timer function, although in my experience, the times 17:00 and 07:00 the next day are not very profitable trading times with many false signals, BUT, to have the right timer is obviously best,

No need to rush on your assessment , you have more important things to do, I am still learning to think like a computer 
#4 - December 19, 2018, 11:41:34 PM

Administrator
  • Hero Member
  • Posts: 4732
  • Points: 34629
  • Likes Received: 4838
  • Reputation: +220/-14
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.
Code: [Select]
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.
Code: [Select]
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);
 }

#5 - December 20, 2018, 12:22:20 AM

  • Sr. Member
  • Posts: 352
  • Points: 71
  • Likes Received: 150
  • Reputation: +11/-0
Thanks for the new Timer, it really works quite nice.

If you want, please check the updated code

I only updated the timer function, not the other suggestions yet.

PS. I think I left cs on 0==0 before I zipped the file, just change that if the EA closes a sell as soon as it opens.

#6 - December 20, 2018, 11:48:30 PM
Attachments:
« Last Edit: December 21, 2018, 12:00:37 AM by Francoisvs »

Administrator
  • Hero Member
  • Posts: 4732
  • Points: 34629
  • Likes Received: 4838
  • Reputation: +220/-14
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.
Code: [Select]
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.

#7 - December 21, 2018, 11:53:20 PM
« Last Edit: December 21, 2018, 11:54:51 PM by Admin »

  • Sr. Member
  • Posts: 352
  • Points: 71
  • Likes Received: 150
  • Reputation: +11/-0
Here are some more suggestions in addition to any previous recommendations.


  • 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.
Thanks, I will do the adjustements:
On StopLoss etc. If any enters a negative value, they deserve more bad trades than good ones, and no orders placed :)
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?

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 8)
#8 - December 22, 2018, 12:03:11 AM

Administrator
  • Hero Member
  • Posts: 4732
  • Points: 34629
  • Likes Received: 4838
  • Reputation: +220/-14


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 8)
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.






#9 - December 22, 2018, 01:17:33 AM

  • Sr. Member
  • Posts: 352
  • Points: 71
  • Likes Received: 150
  • Reputation: +11/-0

  • 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.



I've set the min lots size as MarketInfo(NULL,MODE_MINLOT), how do I use MarketInfo(NULL,MODE_LOTSTEP)when increasing lots?
On one of the brokers all works fine, because MinLot=0.01 en LotStep=0.01, but the other broker does exactly what you said up here. MinLot is 0.1 and LotStep the same as 0.01, so when the EA places the 1st order at 0.1 Lots and it has to increase the Lot Size, the calculated value is 0.18, but obviously an error is generated because it must be 0.2. Do I just Normalize the double?
#10 - January 08, 2019, 03:41:34 PM

  • Sr. Member
  • Posts: 352
  • Points: 71
  • Likes Received: 150
  • Reputation: +11/-0
I've sorted it out, thanks anyway.
#11 - January 08, 2019, 09:55:11 PM

Administrator
  • Hero Member
  • Posts: 4732
  • Points: 34629
  • Likes Received: 4838
  • Reputation: +220/-14
I've sorted it out, thanks anyway.
I was just about to get back to you. Glad you figured it out.
#12 - January 08, 2019, 11:21:32 PM

  • Sr. Member
  • Posts: 352
  • Points: 71
  • Likes Received: 150
  • Reputation: +11/-0
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  8)
#13 - January 08, 2019, 11:28:05 PM

Administrator
  • Hero Member
  • Posts: 4732
  • Points: 34629
  • Likes Received: 4838
  • Reputation: +220/-14
Here is some sample code which you can use to round up or down to the correct lot size. It checks for all the broker's rules regarding lot sizes and will change the lots accordingly.

Code: [Select]
double Lots(double lots)
  {
   double compatiblelot = 0;
   double minlot = MarketInfo(Symbol(),MODE_MINLOT);
   double maxlot = MarketInfo(Symbol(),MODE_MAXLOT);
   double lotstep = MarketInfo(Symbol(),MODE_LOTSTEP);
   double lotsize = MarketInfo(Symbol(),MODE_LOTSIZE);
  
   if (lots>0 && lotstep>0)compatiblelot = NormalizeDouble(lots/lotstep,0)*lotstep;
   if (compatiblelot>maxlot)compatiblelot=maxlot;
   if (compatiblelot<minlot)compatiblelot=minlot; 
  
   return(compatiblelot);
  }

#14 - January 08, 2019, 11:41:52 PM

  • Sr. Member
  • Posts: 352
  • Points: 71
  • Likes Received: 150
  • Reputation: +11/-0
Thanks, I will pop it into a script and see how to implement it
#15 - January 08, 2019, 11:57:15 PM

Members:

0 Members and 1 Guest are viewing this topic.