Hello,
I have written an EA that trails one bar high or low and opens 2 positions. After that, it should halt and do nothing more. I have backtested and forward tested it and it seems to do the job.
This is my first coding attempt in mql4, and I'm not an experienced programmer by any means. So, I would really love it if I could get some feedback about my code before I do more coding. Attached with this post is a zip file that includes the EA and an include file.
Any helpful comments would be most appreciated
Your VerifyLotSize() doesn't work for all symbols . . .
Check that your OrderSelect() works before you use other functions that rely on it.
NormalizeDouble() is very rarely needed, for example it is not needed in your trailing stop functions, the values you are normalizing are already normalized prices.
In your trailing stop functions you need to take care comparing doubles or you will get error 1 issues. (see a recent thread about this)
You have many functions that do very similar tasks, place an order or close an order, these can be simply combined into a couple of functions, this reduces the testing you have to do from many functions to a few and means you can test more thoroughly in the same time.
You are reporting errors, that is good . . . but once you see the error how will you determine the cause ? you don't know the Bid, Ask, TP, SL, entry price, etc at the time . . . don't just print the error print all the info you will need to go back and determine what caused the issue.
Your VerifyLotSize() doesn't work for all symbols . . .
Check that your OrderSelect() works before you use other functions that rely on it.
NormalizeDouble() is very rarely needed, for example it is not needed in your trailing stop functions, the values you are normalizing are already normalized prices.
In your trailing stop functions you need to take care comparing doubles or you will get error 1 issues. (see a recent thread about this)
You have many functions that do very similar tasks, place an order or close an order, these can be simply combined into a couple of functions, this reduces the testing you have to do from many functions to a few and means you can test more thoroughly in the same time.
You are reporting errors, that is good . . . but once you see the error how will you determine the cause ? you don't know the Bid, Ask, TP, SL, entry price, etc at the time . . . don't just print the error print all the info you will need to go back and determine what caused the issue.
Thank you for your comments.
I don't understand why OrderSelect() might not work, and don't see any issues with the way it is called in my code.
As for reporting errors, I have this code in my order opening functions:
string ErrLog = StringConcatenate("Bid: ",MarketInfo(argSymbol,MODE_BID)," Ask: ",MarketInfo(argSymbol,MODE_ASK)," Lots: ",argLotSize); Print(ErrLog);
and this in my stop adjusting functions:
string ErrLog = StringConcatenate("Bid: ",MarketInfo(OrderSymbol(),MODE_BID)," Ask: ",MarketInfo(OrderSymbol(),MODE_ASK)," Ticket: ",argTicket," Stop: ",argStopLoss," Profit: ",argTakeProfit); Print(ErrLog);
isn't that enough?
I replaced all global variables by static variables inside start() and I removed init(). Is this good --or at least better -- practice?
attached is the new mq4
Thank you for your comments.
I don't understand why OrderSelect() might not work, and don't see any issues with the way it is called in my code.
The point is . . . in the event that it does fail how will the code that follows perform ?
As for reporting errors, I have this code in my order opening functions:
and this in my stop adjusting functions:
isn't that enough?
When you output double values for 5 digit pairs you will be missing the 5th digit, this 5th digit may hold the information you need to determine the cause of an issue.
Depends on use and intent. Do you handle deinit/init cycles? Do you handle mid bar?
external static variable - MQL4 forum
https://www.mql5.com/en/forum/146192/page3#825991
RaptorUK knows I disagree with that. NormalizeDouble is NEVER needed. It's a kludge, don't use it. It's use is always wrong.
- Normallizing Price for pending orders must be a multiple of ticksize, metals are multiple of 0.25 not a power of ten.
double NormalizePrice(double p, string pair=""){ // https://forum.mql4.com/43064#515262 zzuegg reports for non-currency DE30: // MarketInfo(chart.symbol,MODE_TICKSIZE) returns 0.5 // MarketInfo(chart.symbol,MODE_DIGITS) return 1 // Point = 0.1 // Prices to open must be a multiple of ticksize if (pair == "") pair = Symbol(); double ts = MarketInfo(pair, MODE_TICKSIZE) return( MathRound(p/ts) * ts ); }
- For TP/SL those are levels, once the market reaches the level, they become a market order. No normalize needed.
- Normallizing lotsize must be a multiple of lotstep. Is lotstep a multiple of ten on all brokers?
double NormalizeLots(double lots, string pair=""){ if (pair == "") pair = Symbol(); double lotStep = MarketInfo(pair, MODE_LOTSTEP), minLot = MarketInfo(pair, MODE_MINLOT); lots = MathRound(lots/ls) * ls; if (lots < minLot) lots = 0; // or minLot return(lots); }
- Comparing doubles, normalize is unnecessary. All that does is hide the fundamental problem, floating point round off. The == operand. - MQL4 forum
string PriceToStr(double p){ return( DoubleToStr(p, Digits) ); }
string PriceToStr(double p){ return( DoubleToStr(p, Digits) ); }
I have a similar function . . .
//+--------------------------------------------------------------------------------+ //| ToStr function - calls DoubleToString (double, Digits) and returns the string | //+--------------------------------------------------------------------------------+ string ToStr(double ValueToString) { return (DoubleToStr(ValueToString, Digits)); }
int ltsDigits; void OnInitLTS(){ ltsDigits=DigitsDouble(market.lotStep); } string LotsToStr(double lots){ return( DoubleToStr(lots, ltsDigits) ); } int DigitsDouble(double var, int maxD=8){ string value = DoubleToStr(var, maxD); // =0.01000000 int iRight = StringLen(value)-1; while(StringGetChar(value, iRight) == '0'){ maxD--; iRight--; } // = 0.01 return(maxD); // 2 }Comment is wrong, doesn't call DoubleToString. I have a function DoubleToString, similar to DoubleToStr but doesn't need a digits arg:
string DoubleToString(double v, int d=8){ // 1.60000000 -> 1.6 string value = DoubleToStr(v, d); for(int iRight = StringLen(value)-1; true; iRight--){ int c = StringGetChar(value, iRight); if(c != '0'){ if(c == '.') iRight--; break; } } return( StringSubstr(value, 0, iRight+1) ); }
- Free trading apps
- Over 8,000 signals for copying
- Economic news for exploring financial markets
You agree to website policy and terms of use
Hello,
I have written an EA that trails one bar high or low and opens 2 positions. After that, it should halt and do nothing more. I have backtested and forward tested it and it seems to do the job.
This is my first coding attempt in mql4, and I'm not an experienced programmer by any means. So, I would really love it if I could get some feedback about my code before I do more coding. Attached with this post is a zip file that includes the EA and an include file.
Any helpful comments would be most appreciated