I seem to be having an issue. Did I no something wrong here? Bare in mind this is draft code. It will look bad.
LOOP:
for each ap-ledger where
ap-ledger.company = "ffi" and
ap-ledger.bank-acc = "99" and
ap-ledger.trans-date = 05/26/2016 and
no-lock
break by ap-ledger.vend-num:
if ap-ledger.ap-amt-type <> "p" then next.
find vendor where vendor.vend-number = ap-ledger.vend-number no-lock no-error.
if not avail vendor then next LOOP.
if vendor.vend-name <> "Jeffery Lewis" then next LOOP. /* */
display ap-ledger with 2 columns 1 down.
temp-amount = temp-amount + ap-ledger.amount.
if ap-ledger.amount < 0 then asd-count = asd-count + 1.
end.
if last-of(ap-ledger.vend-num) then do:
if asd-count > 0 then
final-amount = tmp-amount.
else final-amount = ap-ledger.amount.
final-amount = 0.
asd-count = 0.
end.
end.
To me it looks like the END in this block here:
temp-amount = temp-amount + ap-ledger.amount.
if ap-ledger.amount < 0 then asd-count = asd-count + 1.
end.
terminates the FOR EACH so that the IF LAST-OF (ap-ledget.vend-num) is outside of the FOR EACH with the break by.
And there appears to be an extra "and" just before the no-lock...
That was the problem. I've been rearranging and rewriting this code for about an hour now. So that must have been a left-behind stray "end" . I was going crazy because I could write a much smaller block of code using the same "break by" and it would work.
Thanks
Thanks Fred. The reason there is an extra "and" is because I was editing the code after putting it here, and left that there. It's not in the code editor. As Mike pointed out, there was a premature "end" in my code
You must change your code, in some cases the last-of() could be skipped.
Example: What if the last-of(ap-ledger.vend-num) register have not a "p" on the ap-amt-type? You are doing a "next" and the last-of will never be executed.
Usually I have 2 solutions to that issue, include all the conditionals in the "for each", or create a block and instead next, do a leave, example:
inside1L:
DO:
if ap-ledger.ap-amt-type <> "p" then LEAVE inside1L.
.... more code ....
END.
if last-of(ap-ledger.vend-num) then do:
... more code ....
Good observation, Marco!
I apologize, Joseph, for using your draft code to make the below points... coding-style is just a pet-peeve of mine... I do hope, though, that maybe the one or the other reader might pick up a point or too, or at least start thinking about it in a different way, as it would surely make their life easier... :)
When I was at University, many moons ago, they taught us the concept of a Dykstra Diagram - basically any block should have only one entrance and one exit. I adhere to that ideology all my career-life. And it has saved me from many bugs like the one Marco pointed out. These things are sooooo hard to find, as sometimes stuff works, and sometimes it doesn't...
So, my recommendation is to use IF THEN ELSE blocks, and always make things go through the whole block until the very end. If it is conditional, place it in a THEN or ELSE block...
Also, being a nitpicking virgo, I choose to always comment my END statements - that way I can easily see where one block ends and where the next. And, together with careful indentation, I easily see if there's an end too much or too little, and where. Again, saved me from many bugs. Or, the other way around, found many bugs in others code due to not adhering to these simple best practices...
I'd start the loop like this:
for each ap-ledger no-lock
where ap-ledger.company = "ffi"
and ap-ledger.bank-acc = "99"
and ap-ledger.trans-date = 05/26/2016
break by ap-ledger.vend-num
:
<whatever code>
if last-of(ap-ledger.vend-num)
then do: /* last-of vend-num */
if asd-count > 0
then final-amount = tmp-amount.
else final-amount = ap-ledger.amount.
final-amount = 0.
asd-count = 0.
end. /* last-of vend-num */
end. /* for each ap-ledger no-lock */
Placing WHERE, AND and OR underneath each other, it becomes clear what the conditions structure is, and it is easy to put another line in, or take one out. Placing the THEN into the same column as the ELSE makes it easier to associate the ones that belong together.
And, of course, you realized already, that you set the final-amount to 0 before doing anything with it, in the LAST-OF() THEN block...
I actually caught that while testing. Thank for suggesting it though. Shows you know your stuff!
I ended up moving the " <> "p" " to my WHERE at the end.
This was draft code. Which is why I never actually used "final-amt" I just opened the terminal and thrown in some code. I was trying to implement some ideas off the top of my head to solve a ongoing issue.
Needless to say, after a few more hours of writing and enhancing the code above, I have created a well done solution that worked in the said program I was fixing.
the <> "p" and vendor ...