PDA

View Full Version : Recommended coding practice



Mike Peat
18-Jun-2018, 03:47 AM
Not wanting to add to the "Splitting an if/else/case line" thread here (already quite long) I am starting new.

In the past the recommendation for a situation where what you wanted was a conditional one-liner:


If (condition) DoThing


Was to write:


If (condition) Begin
DoThing
End


However we can now write (obviously we always could, but now the debugger handles it better):


If (condition) ;
DoThing


In the light of that debugger improvement, has the recommendation changed, or is Begin/End still the preferred approach?

Mike

Russell McDougall
18-Jun-2018, 05:18 AM
Don't know but I always used Begin End Block which I felt made Else look more logical to track down rare instances if I had an unmatched Else somewhere which created havoc.

Like
If (Test) begin
do something
End

Some other code
Else do something else

Stephen W. Meeley
18-Jun-2018, 05:37 AM
Mike,

From the help...

Using Begin/End blocks is now only one method of structuring your conditional code to allow for improved debugging. Now, if you prefer to use a single line If/Else/Case (i.e.., no Begin/End block) we recommend that that you break the line using the “;” separator. This makes it much easier to debug the line.
...so it really comes down to using whichever method suites you and the situation. In our own code we use a combination of both (usually based on the complexity of the conditional code).

phvwijk
18-Jun-2018, 11:44 AM
Mike,

I just prefer always the full begin and end.

Consistent.
You always know where to look
If for what ever reason the one-liner becomes more then a one liner

So had not a lot to changes to do for this warning.

Peter

Nils G. Svedmyr
18-Jun-2018, 12:09 PM
I agree with Peter. I do the same.

John Tuohy
18-Jun-2018, 03:16 PM
Mike and All,

As Stephen stated, the single line if thing is a preference and not even a recommendation anymore. This particular feature is a good example of the best of intentions gone astray. Some history might help.

For a number of years we've lightly recommended that you always use if/else with begin/end blocks. We did this because it made it easier to debug code. At DISD this year we talked about compiler warnings. Peter asked about Begin/End warnings and he said he and everyone he talked to would like to see this. We planned on doing this, but it kind of bothered us because single line if/else statements are such a common language construct and we were recommending this based on a debugger limitation. So we looked to see if we could make "single" line if/else statements debug-able when they are broken with a ";". We succeeded so at this point the choice or using a single ";" line of blocks are both equally good. Once implemented we wanted to switch all of our code over so the existing single lines were debug-able. That's why we put the warning in - we found all (or most) of these in our code and changed them. We did this all the way, including case statements. Even if the code is simple (e.g., if bOk Procedure_Return) we split it. You really never know when the simplest of constructs is the source of a bug. We figured that this was such a great new feature that the warning would be welcomed - it is showing you every line of code in an application that cannot be easily debugged. We figured wrong.

Immediately at EDUC this became a high volume source of warnings and controversy. I sure never saw that coming. What's too bad is this somewhat obscured the fact that the change of the breakpoint logic is a really nice addition. If you had been using those ";" line breaks all of those lines could now support breakpoints. That nice addition, got lost in the warning discussion.

Anyway, we listened. A number of you made valid points that this is not an obsolete technique and we were imposing a style preference on everyone else. You just can't tell programmers to do this - it's like telling them they have to sepll properly. Point taken. In the next alpha, the if/else warning will be disabled by default. If you choose you can enable it.

In terms of a recommendation, here is what I would do (that's me personally and not DAW).

1. I'd leave all of my existing if/else/case code as is.
2. If I found myself needing to debug an old single line If, I'd switch it over to a ";" and recompile as needed.
3. I'd never create new single line if statements (always use ";" or begin/end).
4. When appropriate, I'd probably create new "single" lines with a ";" instead of begin/end. That is entirely a matter of personal preference - it doesn't matter.

-John

chuckatkinson
18-Jun-2018, 07:05 PM
Thanks for the clarification. I am already getting ready while editing code that I find the single line and at least putting in the ";" to break the line. My previous coding practice has always been to use Begin/End based on one of your presentations years ago on how to make debugging easier. Alas I have to work on other developers code who did not adhere to this practice.

Mike Peat
19-Jun-2018, 02:38 AM
John

Be not downcast - I think the ability to debug split lines is a great new feature! It helps not just in the if/else/case one-liner situation, but (and IMO actually more importantly) in complex conditionals like:


If (((condition1) and ;
(condition2)) or ;
((condition3) and ;
(condition4))) Begin ... // Think I got all those parens right! <g>


The old way, if something was not right in there, I was in a mess. Now I can inspect the problem much better.

I think the furore of protest is a storm in a tea cup (everybody loves to complain). Nobody is being forced to change! I will be leaving (turning) complier warnings on. I need all the help I can get with my coding mistakes. :p

I also broadly agree with your suggestions. I find wrapping Begin/End round a single line, especially if it happens a lot in one lump of code, distracts from the logic - line splitting is a good compromise between brevity and debugability.

Mike

Nils G. Svedmyr
19-Jun-2018, 03:10 AM
debugability


I like that word! :)

Mike Peat
19-Jun-2018, 03:39 AM
Ain't it wonderful how language evolves to fit our needs! ;)

Nils G. Svedmyr
19-Jun-2018, 03:54 AM
Or by an inventive mind :)

To me it seems you used the rules of the English language and "invented" a perfectly sound new word.

Focus
19-Jun-2018, 04:23 AM
I must admit to being slightly intrigued by a case 'one liner'. Do people write code like this ?



Procedure Test
Integer i j
Move 3 to i
Case Begin
Case (i=1) Move 2 to j
Case (i=3) Move 4 to j
Case (i=4) Move 5 to j
Case Else
Move -1 to j
Case End
End_Procedure


In just having a quick play I noticed the above does not execute the same as this


Procedure Test
Integer i j
Move 3 to i
Case Begin
Case (i=1) Move 2 to j
Case (i=3)
Move 4 to j
Case (i=4)
Move 5 to j
Case Else
Move -1 to j
Case End
End_Procedure


The single line case seems to have an implicit break i.e. the same as


Procedure Test
Integer i j
Move 3 to i
Case Begin
Case (i=1) Move 2 to j
Case (i=3)
Move 4 to j
Case Break
Case (i=4)
Move 5 to j
Case Break
Case Else
Move -1 to j
Case End
End_Procedure


Not wanting to start a whole other discussion on case statements :D

Bob Worsley
19-Jun-2018, 07:47 AM
is a storm in a tea cup
Mike, did you mean "tempest in a teapot"? I thought you guys across the pond invented that one... ;)

Garret Mott
20-Jun-2018, 06:51 AM
Are you sure it shouldn't be "debugableness"?

Mike Peat
20-Jun-2018, 07:32 AM
Absolutely not! Good Lord! What are they teaching young people in schools these days? ;)

Focus
20-Jun-2018, 07:37 AM
..... they are not apparently .... least not in the UK .....

https://www.bbc.co.uk/news/technology-44518612

Garret Mott
20-Jun-2018, 09:32 AM
I'm "young people"? Cool!

OK then - "debugableosity"? "Debugability" implies one has such an ability...

[yes, I'm avoiding work]

Bob Worsley
20-Jun-2018, 12:58 PM
Sounds like you're also avoiding English... :p

Garret Mott
20-Jun-2018, 01:09 PM
Moi?

If you were dealing with a BP that has mixed booleans & indicators, procedural code, gosubs, & gotos & gives an "Attempt to Edit a Protected Record" error - wouldn't you?

Bob Worsley
20-Jun-2018, 01:22 PM
I'd probably shoot myself first and I guess that would be avoiding work, yes. But I was referring to "debugableosity"

matthewd
20-Jun-2018, 04:14 PM
What would also help in cases like this is if the Studio had an "auto-decompose" feature to add the individual conditions of a complex IF expression (If Hydras?) like this to the watches window. Then when you break on this line, it is immediately apparent why the IF statement is evaluating to true or false. i.e. in your example, add to the watch window:


condition1
condition2
(condition1 and condition2)
condition3
condition4
(condition 3 and condition 4)
((condition1 and condition2) or (condition3 and condition4))
Though I tend to think for readability and debugability it may be better to move the logic onto separate lines, .e.g.


move (condition1 and condition2) to bMeaningfulCondition1Name
move (condition3 and condition4) to bMeaningfulCondition2Name
if (bMeandingfulCondition1Name or bMeaningfulCondition2Name) ...
Even still with this style of code having a "Decompose IF conditions to Watches" function would be helpful. It would also help in cases where you want to step through code and see where the result of the IF condition is changing before it hits the IF.