iCustom giving unexpected values
- Tristen Shaw: I am trying to make an iCustom call
Do you really expect an answer? There are no mind readers here and our crystal balls are cracked. Always post all relevant code (using Code button) or attach the source file.
How To Ask Questions The Smart Way. (2004)
Be precise and informative about your problemPost your specific version of “Rex” or a link to it.
-
initError = true; return(INIT_SUCCEEDED);
Why are you return success when you have a failure?
- Tristen Shaw: verything seems to work, except for this.
What “this?”
-
for(int i = limit; i < rates_total; i++){ buyBuffer[i] = 0.0; sellBuffer[i] = 0.0;
Your buffers are as-series. Your loop is not.
-
Do you really expect an answer? There are no mind readers here and our crystal balls are cracked. Always post all relevant code (using Code button) or attach the source file.
How To Ask Questions The Smart Way. (2004)
Be precise and informative about your problemPost your specific version of “Rex” or a link to it.
-
Why are you return success when you have a failure?
-
What “this?”
- Your buffers are as-series. Your loop is not.
1. You will have to forgive me, I'm not sure how to post more than 100% of my code. As for the indicator, I have included it below.
2. EDIT: In going through the code, I am going to assume that you are referring to inside the oninit function when it checks for valid handles. As for that, I'm not sure. It's the same as the code Vlad had me read to learn from. Here is the code from his own example:
if(handle_iCustom == INVALID_HANDLE) { //--- tell about the failure and output the error code PrintFormat( "Failed to create handle of the iCustom indicator for the symbol %s/%s, error code %d", Symbol(), EnumToString(Period()), GetLastError()); //--- the indicator is stopped early m_init_error = true; return(INIT_SUCCEEDED); }
As for how it functions, it did indeed catch an error that I had made. So, I don't know why it returns succeed, but it does catch errors.
3. EDIT: Rereading the question, I now realize what you mean by "this." You see, "This" in this context would refer to the aforementioned problem. Namely: "iCustom giving unexpected values."
4. My buffers are set up the same way that they were in Vlads example. And in a previous project I had them set up the same way and I got correct results from my iCustom calls. Why is it causing a problem now? And what would be the proper way to go about fixing it?
As for how it functions, it did indeed catch an error that I had made. So, I don't know why it returns succeed, but it does catch errors.
If the handle is invalid and an error detected, then you should report it as such. You even state it in your comment "the indicator is stopped early", so you should not be returning a state of "INIT_SUCCEEDED".
if(rexHandle == INVALID_HANDLE || atrHandle == INVALID_HANDLE){ //--- tell about the failure and output the error code PrintFormat( "Failed to create handle of the iCustom indicator for the symbol %s/%s, error code %d", Symbol(), EnumToString(Period()), GetLastError()); //--- the indicator is stopped early initError = true; return(INIT_FAILED);
Also, there is no need to keep track of an error variable "initError" if the indicator is supposed to terminate when an error is detected during the initialisation.
Please note that I did not read all of your code or focusing on all the aspects, but just picking out a few things and giving some titbits.
If the handle is invalid and an error detected, then you should report it as such. You even state it in your comment "the indicator is stopped early", so you should not be return a state of "INIT_SUCCEEDED".
That makes sense. However, the code is a direct copy of Vlads example code. Therefore, since he is an experienced coder, I figured there was something I just wasn't understanding and so I left it as he had it.
I will change that in my code though, and I appreciate your friendly input as always. You are a pleasure to learn from.
Just because you copy someone's code, does not mean you should do it without understanding its logic and mechanics. You should strive to fully understand it and why it works (or does not work). You also have to consider its context in the original source code. When you copy it, then your source code becomes the new context and its behaviour should be adapted to the new context.
Also, no matter how good or experienced the coder, we all make mistakes. Don't assume it is always correct. Check it against your own knowledge and understanding and if it does not seem correct, challenge it to find out why?
In MT5 contrary to MT4, the arrays passed as parameters to the OnCalculate event handler are "non-series" arrays, where the oldest element is at index 0 and the most current element at index "rates_total - 1".
However, when copying another indicator's buffer data via CopyBuffer, the start_pos parameter is for handling a "series" array, where the current data is at index "0", but the data copied is based on a "non-series" array. Yes, I know it is all so confusing, and "Why did MetaQuotes do things in such a strange and round-about way?", but it is what it is and it is up to us poor coders to wrap our heads around and try to make sense of it.
EDIT: Please note, that I did not check your code for this. I am just alerting you of a possible problem.
Just because you copy someone's code, does not mean you should do it without understanding its logic and mechanics. You should strive to fully understand it and why it works (or does not work). You also have to consider its context in the original source code. When you copy it, then your source code becomes the new context and its behaviour should be adapted to the new context.
Also, no matter how good or experienced the coder, we all make mistakes. Don't assume it is always correct. Check it against your own knowledge and understanding and if it does not seem correct, challenge it to find out why?
What you said here makes 100% perfect sense, and I couldn't agree with you more.
When I got Vlads example code I went through it line by line and tried to understand as much of it as I could. I then went further by rewriting his code by hand instead of copy and pasting it because that helped my mind walk through the code. I learned a great deal from it and the 'anatomy' of an indicator became much clearer to me compared to when I started.
I did further research finding a couple videos that did a really good job of explaining the buffers and how the rates_total and prev_calculated works and I found that very enlightening.
However, I did not understand all of it. And, I hope people will look at this from my perspective. I'm self taught and sometimes I just don't know what it is that I don't know and need help to understand something. But then, I also don't like posting questions of forums because of how often it's looked down on. It seems like there is no way to win, no matter how you post there is someone that will make you feel like crap because of how you posted, or because they feel it's something that you should have been able to figure out since they understand it.
So, sometimes when I don't understand something but it seems to be working, I just leave it alone because I don't want to annoy anyone by asking for clarification. Of course, then the fact that I didn't get clarification ends up getting me criticized anyway. It just ends up feeling like a no win situation. (Please keep in mind that when I say 'criticized' I'm not referring to you. You always seem very patient and helpful with your responses.)
Now, I get that when I copy this code into a new project, it's a new context. However, it's essentially the same concept in this case. My code is setting up buffers for a couple indicators, filling those buffers with the information from the indicators, and they performing calculations based on that information. So I didn't see why I would need to change the setup of the buffers since they were essentially doing the same thing as before.
For example, in the previous code, I was storing the data from the WAE indicator and then using that information. If there is an issue with the order of the buffer array, why was I getting correct values from it but incorrect values for the REX indicator? It's doing the same thing isn't it?
Ok, lets start at the beginning and take things one step at a time.
The first thing you should do is make sure that the compilation is "flawless" and currently it is not. So lets address this first. By "flawless" I mean, no errors and no warnings.
Currently the REX indicator has one warning:
declaration of 'weight_sum' hides global variable MovingAverages.mqh 222 155
and your code has two warnings:
possible loss of data due to type conversion from 'double' to 'int' TestRexAtr.mq5 183 16 possible loss of data due to type conversion from 'double' to 'int' TestRexAtr.mq5 184 17
Many coders ignore warnings, when they believe that they are not relevant or that the logic is correct. I personally don't like doing that because you can easily let your ego dictate your actions. I go out of my way to make sure that every warning is resolved no matter how trivial they may seem.
In this case, I will let the Rex warning slide for now, but not your code warnings, because I believe they are serious flaws that hide even worse problems.
bool isATRCrossedUp(double mATR, double mPATR){ int mModATR = mATR * 100000; // possible loss of data due to type conversion from 'double' to 'int' int mModPATR = mPATR * 100000; // possible loss of data due to type conversion from 'double' to 'int' if(mModATR >= atrCrossLevel && mModPATR < atrCrossLevel){ return true; } return false; }
The following question are for making you think about the issue and not a form of criticism:
- Why are you converting the numbers to "int" data type?
Is there a particular logic to it or is it a case of "I did not understand it and left it as it was"?
If there is logic to it, why are you not rounding the number first? - Why are you scaling it by a fixed constant of 100000?
Is there a particular logic to it or is it because it makes it more "readable" to you?
Ok, lets start at the beginning and take things one step at a time.
The first thing you should do is make sure that the compilation is "flawless" and currently it is not. So lets address this first. By "flawless" I mean, no errors and no warnings.
Currently the REX indicator has one warning:
and your code has two warnings:
Many coders ignore warnings, when they believe that they are not relevant or that the logic is correct. I personally don't like doing that because you can easily let your ego dictate your actions. I go out of my way to make sure that every warning is resolved no matter how trivial they may seem.
In this case, I will let the Rex warning slide for now, but not your code warnings, because I believe they are serious flaws that hide even worse problems.
The following question are for making you think about the issue and not a form of criticism:
- Why are you converting the numbers to "int" data type?
Is there a particular logic to it or is it a case of "I did not understand it and left it as it was"?
If there is logic to it, why are you not rounding the number first? - Why are you scaling it by a fixed constant of 100000?
Is there a particular logic to it or is it because it makes it more "readable" to you?
My reply seems to have disappeared. Here is a rewrite.
I'm interested to know where you saw the error. My compiler and my meta trader logs show no errors. However I do see the warnings that you mentioned and you are right, I ignored them because I thought that they were unrelated to the issue.
As for both of your questions, the answer is that all of this was done for what I considered to be readability reasons. I timed it by 100000 to make the number more of what I am used to when referring to the ATR and changing it to an Int was because when I refer to an atr value it's in the form of a whole number. Like if I use the atr as a target, the atr value represents the number of points or pips I'm expecting.
I would be very interested to know how this becomes a serious issue. My way of doing things has usually been that I have been trying to resolve all warnings as you say, but I let these ones slide because I was focused on the rex problem and figured that they were not relevant to the issue, which in hindsight was probably an error on my part.
I did not say there were errors. I said their were warnings and listed them for you.
Don't code for "human readability". Code for the "machine". Code for efficiency. Only think about the "human" part in the final stages when plotting or outputting to text.
If you have to scale, then scale by the Tick Size or by the Point Size, because those are the units you would use. Don't just scale by some arbitrary number that you think is appropriate.
Also, ATR as the name says, is an average, and averages do not align to exact whole numbers, even if you scale them by the tIck size or point size. You have to be very wary of this when planning your logic.
However in this case, neither the ATR not the decision logic for the ATR is for "humans". It is only for internal "machine" usage. So make the code efficient. Instead, scale the input parameter. You can supply the parameter in units of "points" or "ticks" and then scale it to price range, given that the iATR data will be price range.
I don't yet know if this is the cause of your indicator malfunction or not, but if you eliminate all problematic code from that get go, then there will be less "suspect" code to have to debug to find issues. If you let the "small" things slide, they can easily build up into a cascade effect leaving you not knowing what is going wrong.
My aim is not to find the bugs for you or to debug it. My aim is to help you clean up the code and its logic to the point where you yourself will see the problem more easily.
That being said, with the aim of reaching the first milestone of having the code "flawlessly" compile, with no errors or warnings, I would like you to tackle this first part, of rewriting the "isATRCrossedUp" function and the handling of the "atrCrossLevel" parameter, so that the code will compile cleanly.
Once that milestone is completed, we will proceed to the next one. Here is my rendition of how I would rewrite this part (using my own style of coding) to resolve the issues mentioned so far. It is not the only solution. It is merely one solution. Each coder will have their own way of looking at things, and mine is just one of many. Please note, that the code still has a "logic bug" in it. I have left it there on purpose so that we can discuss it later. It has to do with the problem of testing for equality between floating point numbers.
// Inputs input uint i_nAtrCrossLevel = 100; // ATR cross level in points // Global variables double g_dbAtrCrossLevel = WRONG_VALUE; // Global variable for ATR cross level in price range // OnInit event handler int OnInit() { if( i_nAtrCrossLevel < 1 ) { Print( "Error: Invalid ATR Cross Level" ); return INIT_PARAMETERS_INCORRECT; }; g_dbAtrCrossLevel = WRONG_VALUE; // Other logic ... return INIT_SUCCEEDED; }; // OnCalculate event handler int OnCalculate( const int rates_total, const int prev_calculated, const datetime &time[], const double &open[], const double &high[], const double &low[], const double &close[], const long &tick_volume[], const long &volume[], const int &spread[] ) { // Make sure we have rates data if( rates_total < 1 ) return 0; // Make sure we have scaled the input parameter to price range if( g_dbAtrCrossLevel < 0 ) g_dbAtrCrossLevel = i_nAtrCrossLevel * _Point; // Other logic ... // Return value of prev_calculated for next call return rates_total; }; // Support function for ATR cross detection bool isATRCrossedUp( double dbATRcur, double dbATRprev ) { return ( dbATRcur >= g_dbAtrCrossLevel ) && ( dbATRprev < g_dbAtrCrossLevel ); };
- 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 all,
I am trying to make an iCustom call to the REX indicator to retrieve the rex line and the signal line. I even looked at the REX source code to confirm that index 0 and 1 are the correct indices.
I've followed all of my variables down through my code and printed them and everything seems to work, except for this.
No errors
As always, I appreciate any help anyone can offer.