PDA

View Full Version : False compile warning Alpha 3 if/else/case line



Nils G. Svedmyr
26-Jun-2018, 01:39 PM
Alpha 3

I put this at the top of a program;
#IF (!@ > 190)
CompilerLevelWarning All On
#ENDIF

and it gives a compiler warning ;"Splitting an if/else/case line with a delimiter..." warning on the line;

If (iCtr = iLines and sLine = "") Break



As it is a Break COMMAND that follows I think this is a legit construct and shouldn't give a warning.

Further more Wil van Antwerpens update of the CleanMarkers tool does not touch constructs like that, which makes sense to me.

Could this please be fixed DAW?

Stephen W. Meeley
26-Jun-2018, 02:29 PM
Nils,

Actually, both Case and Break are included to trigger the warning. Even before we implemented the warning the example used in the documentation of the Break command shows it being done that way...


Procedure OnClick
Integer iCount // a counter
String sValue // contents of a string
String sTest // test character

// loop through the characters of the string 'hello world'
Move "hello world" To sValue
For iCount From 1 To (length(sValue))
Move (Mid(sValue, 1, iCount)) To sTest
// if the character 'w' is found, quit the loop.
If (sTest = 'w');
Break
Else; // show progress
Showln "Character number " iCount " is a [" sTest "]"
Loop
End_Procedure

Nils G. Svedmyr
26-Jun-2018, 03:08 PM
Right, I get it. I should have thought of splitting the line up with a semicolon - thanks!

On a related issue;

As you can understand it can be very tedious to go through old code and you get tons of compiler warnings. If it's me to blame - fine, and I will deal with them.

But what really gets me and have put me to the brink of giving up fixing old code is when it time after time the references are for DAW packages.

Just now I found problems with "cAnimation.pkg"

Why can't all packages in \Pkg be cleaned up?

Stephen W. Meeley
26-Jun-2018, 03:17 PM
Nils,

it is our intent to make sure that all packages (except those that are obsolete) are fully cleaned and we have not completed that process yet (it's only Alpha :cool:). In fact (and as David Letterman would say "I swear I'm not making this up") I just did more cleaning a few hours ago, including cAnnimation.pkg. We'll get there...

Nils G. Svedmyr
26-Jun-2018, 03:20 PM
Good to know, thanks!

Stephen W. Meeley
26-Jun-2018, 03:24 PM
BTW, if you come across any we've missed - we certainly don't mind getting a gentle reminder about them.

wila
26-Jun-2018, 04:37 PM
Further more Wil van Antwerpens update of the CleanMarkers tool does not touch constructs like that, which makes sense to me.

I'm sure you know already, but for others reading along.

The reason the cleanmarkers tool does not touch this construct is by intent.

While the tool could fix it up by inserting a semi column and a line break, inserting a begin/end block would render your code invalid as the break command then lives in another control block.

Using the break command is a legit language construct, but it isn't the clearest way and can easily break (pun intended) your logic by extending/adjusting your code. Such as by inserting a begin/end block around the break command or even higher up.

eg.


for iItem from 0 to 100
...
Move 10 To b
if (a=10 and iItem=16) Break
..
loop


a reasonably obvious mistake is this one:




for iItem from 0 to 100
...
Move 10 To b
if (a=10 and iItem=16) Begin
Break
end
..
loop



less obvious is this:


for iItem from 0 to 100
...
if (c>10) Begin
Move 10 To b
if (a=10 and iItem=16) Break
end
..
loop





As such I do believe that getting a warning from Studio is the best way to draw attention to it and give you an opportunity to eliminate the break command and use an alternative mechanism to break out of that control block.
For example by introducing an additional boolean variable such as bStop in a while loop and setting that variable to true, or in a for loop such as above by moving a "stop loop" value to iItem.

Now I notice that I picked a bit of a unlucky example as a for loop requires a few more lines to refactor as adjusting the condition and adding a stop boolean variable such as you can in a while loop.

But this would do it:



for iItem from 0 to 100
...
Move 10 To b
if (a=10 and iItem=16) Begin
Move 101 to iItem // stop the loop
end
If (iItem<101) Begin
..
end
loop



This is all a matter of opinion.
Others might argue that the original code is shorter and that it is more clear because of that.
There's some truth in that too and the code is not wrong.

On top of my reasoning for not touching this, it also meant I had more work to handle this special case, which was yet another reason for not doing it (although I could have written the patch to handle it in less time than writing this reply)

--
Wil

Garret Mott
26-Jun-2018, 04:55 PM
I've always liked the Break command & it's sure better than using a GoTo as we once did... ;)

Now to go OT - I'd love stacked Breaks - so we could put them into Begin/End blocks.

Marco
26-Jun-2018, 05:54 PM
GlobMem.pkg as used in
..\DataFlex Reports 6.2\AppSrc\cPrintDialog.h.pkg

Stephen W. Meeley
26-Jun-2018, 08:57 PM
Marco,

Thanks.

wila
27-Jun-2018, 02:50 AM
Garret,

Stacked breaks would be nice and would take away my objections for using the break command if it would take you out of the loop only.
But it would probably break some existing code if they fixed that command.

Btw, my last example could also have been rewritten without using the "overwrite the value of iItem" method, by adjusting the condition and put the remainder of the code in that control block.



for iItem from 0 to 100
...
Move 10 To b
if (a<>10 and iItem<>16) Begin
..
end
loop


Unless you are doing the loop in a time critical area, having the computer count from 16 to 100 is pretty fast.

Now time to find some coffee and wake up.
--
Wil

Garret Mott
27-Jun-2018, 06:15 AM
I've got my coffee - though not sure it'll wake me up.

You are certainly correct that a confuser will count from 16 to 100 pretty quickly - but 1) it offends my sensibilities (what I have of 'em) & 2) doesn't apply if it's a repeat or while loop.

But - I've taken this thread far enough OT & I don't want Nils to come after me!

Nils G. Svedmyr
27-Jun-2018, 12:07 PM
Watch it Garret, I'm on your tail! :p

Vincent Oorsprong
30-Jun-2018, 01:58 AM
Marco,

You must be using an older library version as it is no longer included.