Code Comments
Programming Forum and web based access to our favorite programming groups.
evaluate false, againSo I wrote some code yesterday using my wonderful new discovery of "evaluate false". Here's the logic. 05 MSC-ISS-COUNTRY PIC 9(03). 88 ISS-COUNTRY-IS-USA VALUE 840. 05 MSC-ACQ-STATE PIC X(02). 88 NON-US-SURCHG-ALLOWED VALUES 'CO' 'CA'. IF ICR-COUNTRY-CODE IS NUMERIC MOVE ICR-COUNTRY-CODE TO MSC-ISS-COUNTRY MOVE STR-REGE-ST TO MSC-ACQ-STATE EVALUATE ISS-COUNTRY-IS-USA OR NON-US-SURCHG-ALLOWED WHEN FALSE MOVE ZERO TO STR-SURCHG-AMT MOVE 'N' TO STR-SURCHG-FLAG END-EVALUATE END-IF. During peer code review the reviewer asserted that this would not work as expected. I then asked him if he believe that the following two sets of code were the same as the first: IF ICR-COUNTRY-CODE IS NUMERIC MOVE ICR-COUNTRY-CODE TO MSC-ISS-COUNTRY MOVE STR-REGE-ST TO MSC-ACQ-STATE IF ISS-COUNTRY-IS-USA OR NON-US-SURCHG-ALLOWED CONTINUE ELSE MOVE ZERO TO STR-SURCHG-AMT MOVE 'N' TO STR-SURCHG-FLAG END-IF END-IF. IF ICR-COUNTRY-CODE IS NUMERIC MOVE ICR-COUNTRY-CODE TO MSC-ISS-COUNTRY MOVE STR-REGE-ST TO MSC-ACQ-STATE EVALUATE ISS-COUNTRY-IS-USA OR NON-US-SURCHG-ALLOWED WHEN TRUE CONTINUE WHEN FALSE MOVE ZERO TO STR-SURCHG-AMT MOVE 'N' TO STR-SURCHG-FLAG END-EVALUATE END-IF. He said he didn't believe so; that they were the same as each other but not the same as the original. When asking why, it turns out he interpreted the original kind of as the following: IF ISS-COUNTRY-USA IS FALSE OR NON-US-SURCHG-ALLOWED IS FALSE MOVE ZERO TO STR-SURCHG-AMT MOVE 'N' TO STR-SURCHG-FLAG END-IF (Assume the above was valid Cobol, which of course it is not.) I set him straight on how it works (it evaluates the expression, getting a single true/false result, and then , but now my concern is that the original statement is not as 'obvious' as I had first hoped. (Of course there is the slight possibility that I am incorrect, but I did test what I believe to be all possible cases and got the results I was expecting.) Opinions? Frank
Post Follow-up to this message
Re: evaluate false, againIn article <46C1779A.6F0F.0085.0@efirstbank.com>, "Frank Swarbrick" <Frank.Swarbrick@efirst bank.com> wrote: >I set him straight on how it works (it evaluates the expression, getting a >single true/false result, and then , but now my concern is that the origina l >statement is not as 'obvious' as I had first hoped. It's not. (IMO) > >(Of course there is the slight possibility that I am incorrect, but I did >test what I believe to be all possible cases and got the results I was >expecting.) No, you're correct. > >Opinions? I would have written the critical part as IF NOT (ISS-COUNTRY-IS-USA OR NON-US-SURCHG-ALLOWED) MOVE ZERO TO STR-SURCHG-AMT MOVE 'N' TO STR-SURCHG-FLAG END-IF or (perhaps better yet) the logically equivalent IF NOT ISS-COUNTRY-IS-USA AND NOT NON-US-SURCHG-ALLOWED MOVE ZERO TO STR-SURCHG-AMT MOVE 'N' TO STR-SURCHG-FLAG END-IF precisely because of the possibility for confusion created by EVALUATE FALSE applied to a compound condition. -- Regards, Doug Miller (alphagat milmac dot com) It's time to throw all their damned tea in the harbor again.
Post Follow-up to this message Re: evaluate false, againOn 14 Aug, 16:36, "Frank Swarbrick" <Frank.Swarbr...@efirstbank.com> wrote: > So I wrote some code yesterday using my wonderful new discovery of "evalua te > false". Here's the logic. > > 05 MSC-ISS-COUNTRY PIC 9(03). > 88 ISS-COUNTRY-IS-USA VALUE 840. > 05 MSC-ACQ-STATE PIC X(02). > 88 NON-US-SURCHG-ALLOWED VALUES 'CO' 'CA'. > > IF ICR-COUNTRY-CODE IS NUMERIC > MOVE ICR-COUNTRY-CODE TO MSC-ISS-COUNTRY > MOVE STR-REGE-ST TO MSC-ACQ-STATE > EVALUATE ISS-COUNTRY-IS-USA > OR NON-US-SURCHG-ALLOWED > WHEN FALSE > MOVE ZERO TO STR-SURCHG-AMT > MOVE 'N' TO STR-SURCHG-FLAG > END-EVALUATE > END-IF. > > During peer code review the reviewer asserted that this would not work as > expected. > I then asked him if he believe that the following two sets of code were th e > same as the first: > > IF ICR-COUNTRY-CODE IS NUMERIC > MOVE ICR-COUNTRY-CODE TO MSC-ISS-COUNTRY > MOVE STR-REGE-ST TO MSC-ACQ-STATE > IF ISS-COUNTRY-IS-USA > OR NON-US-SURCHG-ALLOWED > CONTINUE > ELSE > MOVE ZERO TO STR-SURCHG-AMT > MOVE 'N' TO STR-SURCHG-FLAG > END-IF > END-IF. > > IF ICR-COUNTRY-CODE IS NUMERIC > MOVE ICR-COUNTRY-CODE TO MSC-ISS-COUNTRY > MOVE STR-REGE-ST TO MSC-ACQ-STATE > EVALUATE ISS-COUNTRY-IS-USA > OR NON-US-SURCHG-ALLOWED > WHEN TRUE > CONTINUE > WHEN FALSE > MOVE ZERO TO STR-SURCHG-AMT > MOVE 'N' TO STR-SURCHG-FLAG > END-EVALUATE > END-IF. > > He said he didn't believe so; that they were the same as each other but no t > the same as the original. When asking why, it turns out he interpreted th e > original kind of as the following: > > IF ISS-COUNTRY-USA IS FALSE OR NON-US-SURCHG-ALLOWED IS FALSE > MOVE ZERO TO STR-SURCHG-AMT > MOVE 'N' TO STR-SURCHG-FLAG > END-IF > > (Assume the above was valid Cobol, which of course it is not.) > > I set him straight on how it works (it evaluates the expression, getting a > single true/false result, and then , but now my concern is that the origin al > statement is not as 'obvious' as I had first hoped. > > (Of course there is the slight possibility that I am incorrect, but I did > test what I believe to be all possible cases and got the results I was > expecting.) > > Opinions? > > Frank You have found one of the reasons that I rarely use evaluate - the fact that I don't understand FALSE, etc. Better to simplify the evaluate.
Post Follow-up to this message Re: evaluate false, again"Frank Swarbrick" <Frank.Swarbrick@efirstbank.com> wrote in message news:46C1779A.6F0F.0085.0@efirstbank.com... > So I wrote some code yesterday using my wonderful new discovery of > "evaluate > false". Here's the logic. > > 05 MSC-ISS-COUNTRY PIC 9(03). > 88 ISS-COUNTRY-IS-USA VALUE 840. > 05 MSC-ACQ-STATE PIC X(02). > 88 NON-US-SURCHG-ALLOWED VALUES 'CO' 'CA'. > > IF ICR-COUNTRY-CODE IS NUMERIC > MOVE ICR-COUNTRY-CODE TO MSC-ISS-COUNTRY > MOVE STR-REGE-ST TO MSC-ACQ-STATE > EVALUATE ISS-COUNTRY-IS-USA > OR NON-US-SURCHG-ALLOWED > WHEN FALSE > MOVE ZERO TO STR-SURCHG-AMT > MOVE 'N' TO STR-SURCHG-FLAG > END-EVALUATE > END-IF. > > > During peer code review the reviewer asserted that this would not work as > expected. > I then asked him if he believe that the following two sets of code were > the > same as the first: > > IF ICR-COUNTRY-CODE IS NUMERIC > MOVE ICR-COUNTRY-CODE TO MSC-ISS-COUNTRY > MOVE STR-REGE-ST TO MSC-ACQ-STATE > IF ISS-COUNTRY-IS-USA > OR NON-US-SURCHG-ALLOWED > CONTINUE > ELSE > MOVE ZERO TO STR-SURCHG-AMT > MOVE 'N' TO STR-SURCHG-FLAG > END-IF > END-IF. > > > IF ICR-COUNTRY-CODE IS NUMERIC > MOVE ICR-COUNTRY-CODE TO MSC-ISS-COUNTRY > MOVE STR-REGE-ST TO MSC-ACQ-STATE > EVALUATE ISS-COUNTRY-IS-USA > OR NON-US-SURCHG-ALLOWED > WHEN TRUE > CONTINUE > WHEN FALSE > MOVE ZERO TO STR-SURCHG-AMT > MOVE 'N' TO STR-SURCHG-FLAG > END-EVALUATE > END-IF. > > He said he didn't believe so; that they were the same as each other but > not > the same as the original. When asking why, it turns out he interpreted > the > original kind of as the following: > > IF ISS-COUNTRY-USA IS FALSE OR NON-US-SURCHG-ALLOWED IS FALSE > MOVE ZERO TO STR-SURCHG-AMT > MOVE 'N' TO STR-SURCHG-FLAG > END-IF > > (Assume the above was valid Cobol, which of course it is not.) > > I set him straight on how it works (it evaluates the expression, getting a > single true/false result, and then , but now my concern is that the > original > statement is not as 'obvious' as I had first hoped. No, it isn't. This is a case of adding complexity so you can play with the new toys... :-) (yeah, we've all done it...:-)) The problem here is that it disguises the De Morgan. As soon as you use FALSE with a compound condition, you are into De Morgan territory. Most programmers know De Morgan's Laws (even if they may not recognise them by that name): (A.B)' = A' + B' [NOT (A AND B) = NOT A OR NOT B] De Morgan I (A+B)' = A'.B' [NOT (A OR B) = NOT A AND NOT B] De Morgan II You don't have to be an expert in Boolean Algebra to know that this has tripped up so many programmers and made their programs crash and burn. It is the reason behind the discussion on banning NOT that we saw here recently. (It made me shiver to think that people were seriously suggesting removing NOT from the language, and letting the NOT implicit in an ELSE do the job instead. That's like cutting your head off because you don't like the shape of your mouth, and feeding yourself intravenously instead (assuming head removal wasn't fatal :-))... Just because you consider NOT unsightly doesn't mean that it is, or that it is of no use... ) So, the problem with EVALUATE FALSE and compound conditions is that the De Morgan is implicit and not immediately obvious. (With EVALUATE TRUE there is no De Morgan to worry about, and it is encouraged on many sites; that's why many people are not even aware that you CAN EVALUATE FALSE...) Ban it? Well you could, I s'pose... but that's a bit drastic. Why not make the standard be that people are encouraged to use the simplest possible constructs, and run a few lunchtime courses on Boolean Algebra and simplification? (I've done this in a few places and found these informal sessions well attended and fun. Bring your lunch and lets unravel some convoluted COBOL...) The thing about standards is that people should be comfortable with them. If they understand the risks in using various constructs, then they will use them correctly or choose alternatives. If the aim is to keep things simple, and everyone agrees with that aim, there should be no need to BAN anything. Using a negated EVALUATE , as you have done, where a simple IF would serve the purpose, arguably better, might then be viewed as pretentious, or "antisocial" for that particular site... :-) Doug's solution gets my vote... IF NOT (ISS-COUNTRY-IS-USA OR NON-US-SURCHG-ALLOWED) MOVE ZERO TO STR-SURCHG-AMT MOVE 'N' TO STR-SURCHG-FLAG END-IF This clearly shows the De Morgan. Pete. -- "I used to write COBOL...now I can do anything."
Post Follow-up to this message Re: evaluate false, again"Alistair" <alistair@ld50macca.demon.co.uk> wrote in message news:1187121517.482881.128680@g4g2000hsf.googlegroups.com... > On 14 Aug, 16:36, "Frank Swarbrick" <Frank.Swarbr...@efirstbank.com> > wrote: > > You have found one of the reasons that I rarely use evaluate - the > fact that I don't understand FALSE, etc. Better to simplify the > evaluate. > ...or maybe take up truck driving as a career? Pete. -- "I used to write COBOL...now I can do anything."
Post Follow-up to this message Re: evaluate false, againFrank Swarbrick wrote:
>
>
> During peer code review the reviewer asserted that this would not
> work as expected.
>
> I set him straight on how it works (it evaluates the expression,
> getting a single true/false result, and then , but now my concern is
> that the original statement is not as 'obvious' as I had first hoped.
>
>
> Opinions?
>
A similar thing happened to me. I once wrote a FORTRAN subroutine where each
variable was a different number of dollar signs [i.e., $$$ = SQRT($$**2 +
$$$$) ].
A co-worker ("peer" in your example) expressed mild dissatisfaction with my
style.
I smashed the XXXXer in the mouth and moved on.
Post Follow-up to this message Re: evaluate false, againIn article <5iesc4F3oojb8U1@mid.individual.net>, Pete Dashwood <dashwood@removethis.enternet.co.nz> wrote: > > >"Frank Swarbrick" <Frank.Swarbrick@efirstbank.com> wrote in message >news:46C1779A.6F0F.0085.0@efirstbank.com... This is what I would call A Strong Warning. If 'the reviewer' is a slightly-above-average specimen of programmers on your site then your code's intent is not readily grasped by a slightly-above-average specimen of programmer at your site. Whether this means the code needs to be changed or the programmers' understanding needs to be changed I will leave for others; I will say, simply, that for most situations putting code into Prod, the intent of which is not readily grasped by a slightly-above-average specimen of programmer at your site, is, as some programmers might put it: NOT (A-GOOD-IDEA). [snip] > >No, it isn't. I appear to agree with Mr Dashwood... for reasons stated above. > >This is a case of adding complexity so you can play with the new toys... >:-) (yeah, we've all done it...:-)) I disagree with Mr Dashwood... for reasons stated above. What I believe Mr Dashwood is seeing is what I have called 'Look, Ma, I'm a Programmer!' code, where a programmer learns a new construct and then finds a place to put it. > >The problem here is that it disguises the De Morgan. I disagree with Mr Dashwood again. 'The' problem - if there is, in this instance, a 'the' problem - I see is that a slightly-above-average etc etc. > >Why not make the standard be that people are encouraged to use the simplest >possible constructs, and run a few lunchtime courses on Boolean Algebra and >simplification? Change the program or/and change the programmers, as mentioned above. DD
Post Follow-up to this message Re: evaluate false, againFrank Swarbrick wrote: > So I wrote some code yesterday using my wonderful new discovery of "evalua te > false". Here's the logic. > > 05 MSC-ISS-COUNTRY PIC 9(03). > 88 ISS-COUNTRY-IS-USA VALUE 840. > 05 MSC-ACQ-STATE PIC X(02). > 88 NON-US-SURCHG-ALLOWED VALUES 'CO' 'CA'. > > IF ICR-COUNTRY-CODE IS NUMERIC > MOVE ICR-COUNTRY-CODE TO MSC-ISS-COUNTRY > MOVE STR-REGE-ST TO MSC-ACQ-STATE > EVALUATE ISS-COUNTRY-IS-USA > OR NON-US-SURCHG-ALLOWED > WHEN FALSE > MOVE ZERO TO STR-SURCHG-AMT > MOVE 'N' TO STR-SURCHG-FLAG > END-EVALUATE > END-IF. I think I would say EVALUATE TRUE WHEN ISS-COUNTRY-IS-USA WHEN NON-US-SURCHG-ALLOWED CONTINUE WHEN OTHER MOVE ZERO TO STR-SURCHG-AMT MOVE 'N' TO STR-SURCHG-FLAG END-EVALUATE But, I think your code does what you intend. :) -- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~ / \/ _ o ~ Live from Albuquerque, NM! ~ ~ _ /\ | ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ Business E-mail ~ daniel @ "Business Website" below ~ ~ Business Website ~ http://www.djs-consulting.com ~ ~ Tech Blog ~ http://www.djs-consulting.com/linux/blog ~ ~ Personal E-mail ~ "Personal Blog" as e-mail address ~ ~ Personal Blog ~ http://daniel.summershome.org ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~ GEEKCODE 3.12 GCS/IT d s-:+ a C++ L++ E--- W++ N++ o? K- w$ !O M-- V PS+ PE++ Y? !PGP t+ 5? X+ R* tv b+ DI++ D+ G- e h---- r+++ z++++ "Who is more irrational? A man who believes in a God he doesn't see, or a man who's offended by a God he doesn't believe in?" - Brad Stine
Post Follow-up to this message Re: evaluate false, againOn Aug 15, 3:36 am, "Frank Swarbrick" <Frank.Swarbr...@efirstbank.com> wrote: > So I wrote some code yesterday using my wonderful new discovery of "evalua te > false". Here's the logic. > > 05 MSC-ISS-COUNTRY PIC 9(03). > 88 ISS-COUNTRY-IS-USA VALUE 840. > 05 MSC-ACQ-STATE PIC X(02). > 88 NON-US-SURCHG-ALLOWED VALUES 'CO' 'CA'. > Opinions? I dislike 88 levels and don't use them. I dislike building 'magic numbers' into program code and would have these items in a configuration file or in a database. Your programs may never need to be run outside of USA or even outside your site, but what if the states that allow surcharge (?) change. You then need to schedule maintenance, retesting, peer review, .. Oh well it ensures that you will have some work to do in the future. > EVALUATE ISS-COUNTRY-IS-USA > OR NON-US-SURCHG-ALLOWED It seems to me that CO and CA are actually states of the USA. So from a logical point of view you should be testing if the state is CO or CA only when the country _is_ the USA. Your test is true for any state in the USA or for the states of CA or CO in any country. Therefore it is only FALSE if the country is not America AND the state is not (CA or CO). That doesn't make sense.
Post Follow-up to this message Re: evaluate false, againOn Aug 15, 3:36 am, "Frank Swarbrick" <Frank.Swarbr...@efirstbank.com> wrote: > So I wrote some code yesterday using my wonderful new discovery of "evalua te > false". Here's the logic. > > 05 MSC-ISS-COUNTRY PIC 9(03). > 88 ISS-COUNTRY-IS-USA VALUE 840. > 05 MSC-ACQ-STATE PIC X(02). > 88 NON-US-SURCHG-ALLOWED VALUES 'CO' 'CA'. > > IF ICR-COUNTRY-CODE IS NUMERIC > MOVE ICR-COUNTRY-CODE TO MSC-ISS-COUNTRY > MOVE STR-REGE-ST TO MSC-ACQ-STATE > EVALUATE ISS-COUNTRY-IS-USA > OR NON-US-SURCHG-ALLOWED > WHEN FALSE > MOVE ZERO TO STR-SURCHG-AMT > MOVE 'N' TO STR-SURCHG-FLAG > END-EVALUATE > END-IF. > (Of course there is the slight possibility that I am incorrect, but I did > test what I believe to be all possible cases and got the results I was > expecting.) My testing had your evaluate as above except I had a WHEN TRUE displaying 'surcharge' and WHEN FALSE displaying 'no surcharge' 840CO SURCHARGE 840XX SURCHARGE 990CO SURCHARGE 990XX NO SURCHARGE What does the spec say you should be doing ?
Post Follow-up to this message
Sponsored Links
Powered by vBulletin
Copyright 2000-2006 Jelsoft Enterprises Limited.
