Formatting of 'If' Conditions

Fernando Morales 2018.08.07 10:33 #1

Gherkin:

Hi everyone,

Troubleshooting my EA at the moment and have had some problems with a particular 'if' condition not being activated. I suspect the issue is an improper calculation from the ICustom code that comes before or the way time is expressed as a string "..."

ICustom code (the value of 1000 is used due to the buffers returning EMPTY_VALUE, aka the max double value of 2147483647)

if((BufferONE>1000)&&(BufferTWO>1000))GoLong=true;
if((BufferONE>!1000)&&(BufferTWO>!1000))GoLong=false;

'If' Condition.

if(
   ((DayOfWeek()==5)&&(TimeToStr(TimeCurrent(),TIME_SECONDS)<="22:43:59")&&(GoLong==true))
      ||((GoLong==true)&&(DayOfWeek()==1)&&(TimeToStr(TimeCurrent(),TIME_SECONDS)>="00:30:00"))
      ||((GoLong==true)&&(DayOfWeek()==2))
      ||((GoLong==true)&&(DayOfWeek()==3))
      ||((GoLong==true)&&(DayOfWeek()==4))
)

Can anyone see what's going on here?

Try creating variables to simplify the code

The are parenthesis not grouping conditions

Remember that && has priority over ||

Gherkin 2018.08.07 12:44 #2

Fernando Morales:

Try creating variables to simplify the code

The are parenthesis not grouping conditions

Remember that && has priority over ||

Thanks for the suggestions Fernando.

Is grouping these conditions a matter of removing the individual brackets? So for example

((GoLong==true)&&(DayOfWeek()==3))

Becomes

(GoLong==true&&DayOfWeek()==3)

Or are multiple seperate 'If' conditions linked by (||) needed?

Metatrader 4 coding problem [ARCHIVE!] Any rookie question, [WARNING CLOSED!] Any newbie

Fernando Morales 2018.08.07 13:40 #3

Gherkin:

(GoLong==true&&DayOfWeek()==3)

Yes, this is the way for grouping conditions. Then you can compare the groups.

Also, remember to place on the left the condition which is less likely to change with each incoming tick like DayOfWeek() which will have the same value for a period of 24h.

I can see you use GoLong many times. Perhaps this another option may help:

if ( GoLong ) {
        switch ( DayOfWeek() ) {
                case 1:
		   if (TimeToStr(TimeCurrent(),TIME_SECONDS)>="00:30:00") DoThat;
		   break;
                case 2:
                case 3:
                case 4:
                case 5:	
		   if (TimeToStr(TimeCurrent(),TIME_SECONDS)<="22:43:59") DoThis;
        }
}

Questions from Beginners MQL4 Profit calculation of closed Impulse

Gherkin 2018.08.08 01:19 #4

Fernando Morales:

Yes, this is the way for grouping conditions. Then you can compare the groups.

Also, remember to place on the left the condition which is less likely to change with each incoming tick like DayOfWeek() which will have the same value for a period of 24h.

I can see you use GoLong many times. Perhaps this another option may help:

I see what you mean, a switch operator would definitely be faster for the EA than cycling through all the components of the 'if' condition.

If I've understood your advice correctly there are two options here.

First, your helpful suggestion using the switch operator.

This is perfectly feasible but would involve replicating 'DoThat' and 'DoThis' in case (2), (3) and (4) significantly increasing the amount of code in the EA. In the EA 'DoThis' or 'DoThat' would be about 200-300 lines of code each. Nothing wrong with this approach, only a slight issue of readability.

if ( GoLong ) {
        switch ( DayOfWeek() ) {
                case 1:
		   if (TimeToStr(TimeCurrent(),TIME_SECONDS)>="00:30:00") DoThat;
		   break;
                case 2:
                case 3:
                case 4:
                case 5:	
		   if (TimeToStr(TimeCurrent(),TIME_SECONDS)<="22:43:59") DoThis;
        }
}

Second, removing all the individual brackets from the original code keeping in mind (&&) is executed before (||). This is marginally slower than your option but reduces having to drastically increase the amount of code within the EA.

if(
   (DayOfWeek()==5&&TimeToStr(TimeCurrent(),TIME_SECONDS)<="22:43:59"&&GoLong==true)
      ||(GoLong==true&&DayOfWeek()==1&&TimeToStr(TimeCurrent(),TIME_SECONDS)>="00:30:00")
      ||(GoLong==true&&DayOfWeek()==2)
      ||(GoLong==true&&DayOfWeek()==3)
      ||(GoLong==true&&DayOfWeek()==4)
)

Would both these options produce the same result?

Your pearls of wisdom are of great help Fernando, after a yay or nay on these options I shouldn't be taking up too much more of your time.

Bar close rules Closing with Highest High/Lowest Lesson 4 - MQL4

Fernando Morales 2018.08.08 06:42 #5

This is perfectly feasible but would involve replicating 'DoThat' and 'DoThis' in case (2), (3) and (4) significantly increasing the amount of code in the EA. In the EA 'DoThis' or 'DoThat' would be about 200-300 lines of code each. Nothing wrong with this approach, only a slight issue of readability.

From your comment I understand that the code to run on Mondays is different than on Fridays, but the same code is run from Tuesday to Thursday. I think the Switch operator is your best solution for keeping the readability of the code. Besides, in general, when there is need of repeating the same block of code the practical and elegant solution is creating a function.

This is what I would do:

void OnTick() {
        if ( GoLong ) {
                switch ( DayOfWeek() ) {
                        case 1:
                           if (TimeToStr(TimeCurrent(),TIME_SECONDS)>="00:30:00") MondayTask();
                           break;
                        case 5: 
                           if (TimeToStr(TimeCurrent(),TIME_SECONDS)<="22:43:59") FridayTask();
                           break;
                        default:
                           Tuesday2ThursdayTask();
                }
        }
        else if ( GoShort) {

        }
}

void MondayTask() {
        DoTask1;
}

void FridayTask() {
        DoTask2;
}

void Tuesday2ThursdayTask() {
        DoTask3;
}

In my opinion is worth invest extra time coding with the highest readability for yourself or others to read

Have the open price Assignment operator / default Indicator Miscellaneous Questions

Gherkin 2018.08.08 08:41 #6

Fernando Morales:

From your comment I understand that the code to run on Mondays is different than on Fridays, but the same code is run from Tuesday to Thursday. I think the Switch operator is your best solution for keeping the readability of the code. Besides, in general, when there is need of repeating the same block of code the practical and elegant solution is creating a function.

This is what I would do:

In my opinion is worth invest extra time coding with the highest readability for yourself or others to read

Thank you good sir, that is the solution I'll go with.