Review of code with focus on comments and style

Tristen Shaw 2022.05.16 16:00 #31

Fernando Carreiro #:

Ok, here is continuing our saga ...

Next, some test code to simply copy and display the data obtained from iATR. Put up this indicator and the standard ATR indicator so as to compare the two.

Then look at the code and analyse, question and comment on it.

Don't worry. We will slowly build it up again to the full code you are developing.

Ok, the first thing I notice is that it has an extra decimal place compared to the regular ATR. I have yet to figure out why that is. But, it might effect the the outcome when I was multiplying it by 100,000.

After that, it looks to me like the beginning of your onCalc function is significantly simpler than I have it.

And as always, the entire code looks very clean and easy to look through.

Other than that, it seems to be fairly self explanatory.

Elite indicators :) can anyody point me Error Code "0"?

William Roeder 2022.05.16 16:49 #32

Tristen Shaw #: Ok, the first thing I notice is that it has an extra decimal place compared to the regular ATR. I have yet to figure out why that is.

Floating-point has an infinite number of decimals, it's you, not understanding floating-point and that some numbers can't be represented exactly. (like 1/10.)
Double-precision floating-point format - Wikipedia

If you want to see the correct number of digits, convert it to a string with the correct/wanted accuracy.
question about decima of marketinfo() - MQL4 programming forum (2016)

Curious - can someone Rounding is not working MQL5 : Writing a

Tristen Shaw 2022.05.16 16:55 #33

William Roeder #:

Floating-point has an infinite number of decimals, it's you, not understanding floating-point and that some numbers can't be represented exactly. (like 1/10.)
Double-precision floating-point format - Wikipedia

If you want to see the correct number of digits, convert it to a string with the correct/wanted accuracy.
question about decima of marketinfo() - MQL4 programming forum (2016)

Actually, all I meant was that his ATR indicator had more decimal points showing. I do realize that floating point numbers have an infinite number of decimals. Please forgive me for not being completely specific with my terminology.

Nor was I implying it was a big thing or a problem. I was simply noting the difference between the two indicators because that was what I was asked to do.

Also, I do recognize that some numbers cant be represented exactly, that was one of the last lessons Fernando went over with me. Nor is that related to my observation of more displayed decimal places.

double numbers problem Entry lots value why return many decimal

[Deleted] 2022.05.16 16:59 #34

Tristen Shaw #:Ok, the first thing I notice is that it has an extra decimal place compared to the regular ATR. I have yet to figure out why that is. But, it might effect the the outcome when I was multiplying it by 100,000. After that, it looks to me like the beginning of your onCalc function is significantly simpler than I have it. And as always, the entire code looks very clean and easy to look through. Other than that, it seems to be fairly self explanatory.

No, it does not have extra decimal points. Internally the values are in full double precision floating point. It is only when displayed that we need to consider the number of decimal digits.

For this simple code I have not set the maximum number of visible decimal digits and as a consequence, it is using the default of 8 digits instead of the number of digits of the underlying symbol.

IndicatorSetInteger( INDICATOR_DIGITS, _Digits );

This I will add to the code later, when we start considering the actual plotted buffers and not the calculated ones. Since ATR will be a calculation buffer, I did not bother doing that yet.

Need help, how to Implicit or explicit decimals Discussion on MQL4 documentation

[Deleted] 2022.05.16 17:08 #35

Tristen Shaw #: Actually, all I meant was that his ATR indicator had more decimal points showing. I do realize that floating point numbers have an infinite number of decimals. Please forgive me for not being completely specific with my terminology. Nor was I implying it was a big thing or a problem. I was simply noting the difference between the two indicators because that was what I was asked to do. Also, I do recognize that some numbers cant be represented exactly, that was one of the last lessons Fernando went over with me. Nor is that related to my observation of more displayed decimal places.

Next, since the aim is for you to keep learning and understanding it step by step, I am going to ask you to take my simple code, and modify it to plot the two REX buffers instead of the ATR. Just the two REX plots, nothing else.

After that, I will introduce a few support functions to help clean it up further as we approach your intended objective.

Coding help MetaTrader 5 Client Terminal Previous Bar Trailing Stop

[Deleted] 2022.05.16 17:28 #36

By the way, if you wondering why I use those "weird" prefixes in my variable names, it is to help identify them more easily.

I use "i_" for inputs and "g_" for global variables. I use "a" for arrays, "h" for handles, "n" for integer based numbers, "db" for double floating point, and several others not present in this code.

So "g_adbAtrBuffer" is a global variable of an array of double precision floating point for the ATR buffer, ...

and "i_nAtrPeriod" is a input variable of an integer numeric for the ATR period.

This is not necessary. It is just the coding style I am adopting. You should use what ever style you feel comfortable using that helps you read your code more easily.

How to keep previous Interesting and Humour call for the cooperation

Tristen Shaw 2022.05.16 20:47 #37

Fernando Carreiro #:

By the way, if you wondering why I use those "weird" prefixes in my variable names, it is to help identify them more easily.

I use "i_" for inputs and "g_" for global variables. I use "a" for arrays, "h" for handles, "n" for integer based numbers, "db" for double floating point, and several others not present in this code.

So "g_adbAtrBuffer" is a global variable of an array of double precision floating point for the ATR buffer, ...

and "i_nAtrPeriod" is a input variable of an integer numeric for the ATR period.

This is not necessary. It is just the coding style I am adopting. You should use what ever style you feel comfortable using that helps you read your code more easily.

I figured that's what it was. The only one I didn't figure out was db.

Ok, so I wrote it all over again in my own hand and more or less in my style. Helps me to process the everything mentally.

And it seems to work. :)

// Define indicator properties

   // Define plotting considitions
      #property indicator_separate_window
      #property indicator_buffers 2
      #property indicator_plots   2

   // Define properties for ATR plot
      #property indicator_label1  "Rex"
      #property indicator_type1   DRAW_LINE
      #property indicator_color1  clrGreen
      #property indicator_style1  STYLE_SOLID
      #property indicator_width1  1

      #property indicator_label2  "Signal"
      #property indicator_type2   DRAW_LINE
      #property indicator_color2  clrRed
      #property indicator_style2  STYLE_SOLID
      #property indicator_width2  1

// Declare input variables
   input uint 
      inpRexPeriod = 14,
      inpRexSignal = 14;
      
   input ENUM_MA_METHOD
      inpRexMethod = MODE_SMA,
      inpRexSignalMethod = MODE_SMA;
   
// Declare buffers arrays
   double
      rexBuffer[],
      rexSignalBuffer[];            // Array for ATR buffer
   
// Declare indicator handles
   int 
      rexHandle = INVALID_HANDLE;  // Handle for ATR indicator

// De-initialisation event handler
   void OnDeinit( const int reason ){ 
      ReleaseAllHandles(); 
   };
   
// Initialisation event handler
   int OnInit(){
      // Check parameters
         if( inpRexPeriod < 1 ){
         
            Print( "Error: Invalid REX period" );
            return INIT_PARAMETERS_INCORRECT;
         };
         
         if( inpRexSignal < 1 ){
            
            Print( "Error: Invalid REX Signal" );
            return INIT_PARAMETERS_INCORRECT;
         };
   
      // Obtain indicator handles
         ResetLastError();
         rexHandle = iCustom( _Symbol, _Period, "REX",
                              inpRexPeriod,
                              inpRexMethod,
                              inpRexSignal,
                              inpRexSignalMethod );
                       
      // Check validity of handls
         if( rexHandle == INVALID_HANDLE ){
         
            PrintFormat( "Error %d: Failed to obtain indicator handles for symbol: %s, time-frame: %s",
               _LastError, _Symbol, EnumToString( _Period ) );
            ReleaseAllHandles();
            return INIT_FAILED;
         };
         
      // Set buffers
         SetIndexBuffer( 0, rexBuffer, INDICATOR_DATA );
         SetIndexBuffer( 1, rexSignalBuffer, INDICATOR_DATA );

      return INIT_SUCCEEDED;  // Successful initialisation of indicator
   };

// 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; // No data available yet
         
      // Obtain and check number of calculated bars for ATR indicator
         ResetLastError();
         int nRexBarsCalc = BarsCalculated( rexHandle );         
         if( nRexBarsCalc < rates_total ){
         
            PrintFormat( "Error %d: Insufficient REX indicator data available", _LastError );
            return 0;
         };

      // Calculate bar counting variables
         int
            // Adjust number of previously calculated bars accounting for incomplete current bar
               nPrevCalc    =   prev_calculated > rates_total ? rates_total     - 1
                            : ( prev_calculated > 1           ? prev_calculated - 1 : 0 ),
            // Calculate how many bars need to be updated
               nBarsUpdate  = rates_total - nPrevCalc;
         
      // Collect ATR data
         ResetLastError();
         int nRexBarsCopied = CopyBuffer( rexHandle, 0, 0, nBarsUpdate, rexBuffer );
         int nRexSignalBarsCopies = CopyBuffer( rexHandle, 1, 0, nBarsUpdate, rexSignalBuffer );
         
         if( (nRexBarsCopied < nBarsUpdate) || (nRexSignalBarsCopies < nBarsUpdate) ){
            //Print((nRexBarsCopied < nBarsUpdate), (nRexSignalBarsCopies < nBarsUpdate));
            PrintFormat( "Error %d: Insufficient REX data copied", _LastError );
            return 0;
         };

      return rates_total;  // Return value for prev_calculated of next call
   };
   
   // Support function to release indicator handle
   void ReleaseHandle( int &mHandle, string mName ){
      if( mHandle != INVALID_HANDLE ){
         
         ResetLastError();
         
         if( IndicatorRelease( mHandle ) )
            mHandle = INVALID_HANDLE;
         else
            Print( "Error %d: Unable to release %s indicator handle", _LastError, mName );
      
      };
   };

// Support function to release all indicator handles
   void ReleaseAllHandles( void )
   {
      ReleaseHandle( rexHandle, "REX" );
   };

Pure maths, physics, logic Any questions from newcomers Harmonic Analysis

[Deleted] 2022.05.16 23:15 #38

Tristen Shaw #: I figured that's what it was. The only one I didn't figure out was db. Ok, so I wrote it all over again in my own hand and more or less in my style. Helps me to process the everything mentally. And it seems to work. :)

When writing code, get into the habit of writing proper comments. You changed the code but left the comments still referring to the ATR stuff, instead of updating them to reflect the new context, namely the REX indicator. This may seem inconsequential, but those comments are what will allow you to read the code (especially in the future) and give you a better sense of what it is doing.

Always remember to have comments on the inputs, because they are displayed in the Indicator’s properties/parameters window. You want to know what the parameters are and how to set them, instead of just having some cryptic variable names.

So, as an exercise to improve your code readability, change the ATR comments, add comments to the parameters, and add new comments to those sections of code that seem cryptic to you so that you can remind yourself of what they do.

If, however, you decide you don’t want to do, then let me know, and I will move on with the next part.

By the way, I use “db” for double and “dt” for datetime; hence the reason I did not simply use a single character such as “d”.

Delete an object from How to pass arrays Ask!

Tristen Shaw 2022.05.16 23:56 #39

Fernando Carreiro #:

When writing code, get into the habit of writing proper comments. You changed the code but left the comments still referring to the ATR stuff, instead of updating them to reflect the new context, namely the REX indicator. This may seem inconsequential, but those comments are what will allow you to read the code (especially in the future) and give you a better sense of what it is doing.

Always remember to have comments on the inputs, because they are displayed in the Indicator’s properties/parameters window. You want to know what the parameters are and how to set them, instead of just having some cryptic variable names.

So, as an exercise to improve your code readability, change the ATR comments, add comments to the parameters, and add new comments to those sections of code that seem cryptic to you so that you can remind yourself of what they do.

If, however, you decide you don’t want to do, then let me know, and I will move on with the next part.

By the way, I use “db” for double and “dt” for datetime; hence the reason I did not simply use a single character such as “d”.

Going to do this right. Not skipping. :)

// Define indicator properties

   // Define plotting considitions
      #property indicator_separate_window
      #property indicator_buffers 2
      #property indicator_plots   2

   // Define properties for REX plot
      #property indicator_label1  "Rex"
      #property indicator_type1   DRAW_LINE
      #property indicator_color1  clrGreen
      #property indicator_style1  STYLE_SOLID
      #property indicator_width1  1

      #property indicator_label2  "Signal"
      #property indicator_type2   DRAW_LINE
      #property indicator_color2  clrRed
      #property indicator_style2  STYLE_SOLID
      #property indicator_width2  1

// Declare input variables
   input uint 
      inpRexPeriod =       14,               // Rex Period
      inpRexSignal =       14;               // Signal Period
      
   input ENUM_MA_METHOD
      inpRexMethod =       MODE_SMA,         //Rex Method
      inpRexSignalMethod = MODE_SMA;         //Signal Method
   
// Declare buffers arrays
   double
      rexBuffer[],                           // Array for Rex buffer
      rexSignalBuffer[];                     // Array for REX signal buffer
   
// Declare indicator handles
   int 
      rexHandle =          INVALID_HANDLE;   // Handle for REX indicator

// De-initialisation event handler
   void OnDeinit( const int reason ){ 
      ReleaseAllHandles(); 
   };
   
// Initialisation event handler
   int OnInit(){
      // Check parameters
         if( inpRexPeriod < 1 ){
         
            Print( "Error: Invalid REX period" );
            return INIT_PARAMETERS_INCORRECT;
         };
         
         if( inpRexSignal < 1 ){
            
            Print( "Error: Invalid REX Signal" );
            return INIT_PARAMETERS_INCORRECT;
         };
   
      // Obtain indicator handles
         ResetLastError();
         rexHandle = iCustom( _Symbol, _Period, "REX",
                              inpRexPeriod,
                              inpRexMethod,
                              inpRexSignal,
                              inpRexSignalMethod );
                       
      // Check validity of handles
         if( rexHandle == INVALID_HANDLE ){
         
            PrintFormat( "Error %d: Failed to obtain indicator handles for symbol: %s, time-frame: %s",
               _LastError, _Symbol, EnumToString( _Period ) );
            ReleaseAllHandles();
            return INIT_FAILED;
         };
         
      // Set buffers
         SetIndexBuffer( 0, rexBuffer,       INDICATOR_DATA );
         SetIndexBuffer( 1, rexSignalBuffer, INDICATOR_DATA );

      return INIT_SUCCEEDED;  // Successful initialisation of indicator
   };

// 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; // No data available yet
         
      // Obtain and check number of calculated bars for REX indicator
         ResetLastError();
         int nRexBarsCalc = BarsCalculated( rexHandle );         
         if( nRexBarsCalc < rates_total ){
         
            PrintFormat( "Error %d: Insufficient REX indicator data available", _LastError );
            return 0;
         };

      // Calculate bar counting variables
         int
            // Adjust number of previously calculated bars accounting for incomplete current bar
               nPrevCalc    =   prev_calculated > rates_total ? rates_total     - 1
                            : ( prev_calculated > 1           ? prev_calculated - 1 : 0 ),
            // Calculate how many bars need to be updated
               nBarsUpdate  = rates_total - nPrevCalc;
         
      // Collect REX data
         ResetLastError();
         int nRexBarsCopied = CopyBuffer( rexHandle, 0, 0, nBarsUpdate, rexBuffer );
         int nRexSignalBarsCopies = CopyBuffer( rexHandle, 1, 0, nBarsUpdate, rexSignalBuffer );
         
         // Check to see if we have enough indicator data
         if( (nRexBarsCopied < nBarsUpdate) || (nRexSignalBarsCopies < nBarsUpdate) ){
           
            PrintFormat( "Error %d: Insufficient REX data copied", _LastError );
            return 0;
         };

      return rates_total;  // Return value for prev_calculated of next call
   };
   
   // Support function to release indicator handle
   void ReleaseHandle( int &mHandle, string mName ){
      if( mHandle != INVALID_HANDLE ){
         
         ResetLastError();
         
         if( IndicatorRelease( mHandle ) )
            mHandle = INVALID_HANDLE;
         else
            Print( "Error %d: Unable to release %s indicator handle", _LastError, mName );
      
      };
   };

// Support function to release all indicator handles
   void ReleaseAllHandles( void )
   {
      ReleaseHandle( rexHandle, "REX" );
   };

Vladimir Karputov 2022.05.17 11:15 #40

What problems? Need my help?

iCustom giving unexpected values - page 4