Level Extreme platform
Subscription
Corporate profile
Products & Services
Support
Legal
Français
One exit per procedure/function/codeblock to what purpos
Message
General information
Forum:
Visual FoxPro
Category:
Other
Miscellaneous
Thread ID:
00835552
Message ID:
00836567
Views:
19
Jim,

That's not my assertion here.

I *agree* with your statement that having to check a return value EACH TIME you want to do something is not a good approach. I would go further and assert that if the developer is writing code of that nature, the module should be re-factored (or at least re-thought).

Without nit-picking Mike's example to death (bg)(ducking), what I'm suggesting IN THE SAMPLE HE PROVIDES is that is doesn't buy him anything to have the DO CLEANUP -- RETURN .f. at the end of the first case, when NONE of the other cases uses an intermediate RETURN and he performs the DO CLEANUP at the end of the DO CASE anyway. That's all.

BTW -- I don't consider it "bad" to code it that way -- just a different "personal style" to which I don't (necessarily) subscribe. Besides, I don't dare call Mike's code "bad" anyway, since he can run faster than I can...

Evan


>Hi Evan,
>
>The thing I don't 'get' is... why would it be considered 'bad' to code
...
>do Cleanup
>Return .F.
>ENDIF
>
>I personally wouldn't care if that sequence was coded 50 times in a program. What I *would* care about is having
...
>llReturn = .F.
>ENDIF
followed by EXTRA code that has to check llReturn every time something new was to be done.
>
>Admittedly, all personal taste.
>cheers
>
>>Hi Mike,
>>
>>Excellent example.
>>
>>After discussing this further with yourself and others here in this forum, the only exit point in your "better" sample with which I would take further issue would be the RETURN .f. in the first CASE statement. Since you DO CLEANUP at the end of the CASE anyway, why specify it again there just to be able to RETURN right away rather than set a flag? Okay, I *KNOW* I'm nit picking here -- I'm using your example to show my point -- also, another (future) developer, seeing this style used, might use this elsewhere in the system where many more CASEs existed.
>>
>>As long as the multiple exit points occur at the TOP of the module, before the real "meat" of the processing occurs, I can live with that being one of the "de facto" coding standards. Where I have the difficulty is scattering these exit points throughout code, without a solid reason (read: lazy programming) or sufficient documentation as to the reason (every rule can be broken IF YOU HAVE A DOCUMENTABLE REASON TO DO SO) and/or making a blanket statement (which is what began this discussion) that the "single exit point" practice should be retired.
>>
>>I appreciate your efforts in finding and sharing these code samples here. Great discussion!
>>
>>Evan
>>
>>>Hi Evan
>>>
>>>Here's some original code. It is very old. I only removed filenames
>>>
>>>
>>>IF XNETUSE("FILEA") .AND. ;
>>>  XNETUSE("FILEB") .AND. ;
>>>  XNETUSE("FILEC")
>>>   SELECT FILEC
>>>   OK = .T.
>>>   IF RECCOUNT() > 0
>>>      SUM amount TO dollars
>>>      @ 6,0  CLEAR TO 20,79
>>>      action = 'A'
>>>
>>>      @ 12,10 SAY XASTR(RECCOUNT()) + " Records to Append - " + ;
>>>          "Value $ " + ALLTRIM(TRANSFORM(m.dollars,'9,999,999,999.99'))
>>>      @ 13,10 SAY "Append, Delete, or Cancel? (A/D/C) " ;
>>>	      GET action PICTURE "!" VALID action $ "ADC"
>>>      READ
>>>      DO CASE
>>>      CASE action = 'A'
>>>         DO XCENTER With "Appending DATA",15
>>>
>>>         ON ERROR OK = .F.
>>>         SELECT FILEA
>>>         GO TOP
>>>         DO WHILE NOT EOF()
>>>           SCATTER MEMVAR MEMO
>>>           SELECT FILEC
>>>           APPEND BLANK
>>>           GATHER MEMO MEMVAR
>>>           SELECT FILEA
>>>           SKIP
>>>         ENDDO
>>>
>>>         SELECT FILEA
>>>         ZAP
>>>
>>>         SELECT FILEB
>>>
>>>         IF OK
>>>           REPLACE ALL appended WITH .T. FOR appended = .F.
>>>          ELSE
>>>           DO XCENTER WITH ;
>>>               "Append failed! "+;
>>>               "Deleting all loaded data.",15
>>>           DELETE FOR appended = .F.
>>>           PACK
>>>         ENDIF
>>>
>>>      CASE action = 'D'
>>>         DO XCENTER With "Deleting all loaded data.",15
>>>         SELECT FILEC
>>>         ZAP
>>>
>>>         SELECT FILEB
>>>         DELETE FOR appended = .F.
>>>         PACK
>>>
>>>      CASE action = 'C'
>>>         DO XCENTER With "Cancelled - data remains",15
>>>         READ
>>>      ENDCASE
>>>   ELSE
>>>      DO XCENTER With "There is no data to Append",15
>>>      READ
>>>   ENDIF
>>>ENDIF
>>>RETURN OK
>>>
>>>
>>>
>>>***************************************************
>>>Now seriously! How is that any better than this? ...
>>>***************************************************
>>>
>>>
>>>IF NOT (XNETUSE("FILEA") .AND. ;
>>>     XNETUSE("FILEB") .AND. ;
>>>     XNETUSE("FILEC"))
>>>
>>>  *Notice there is no failure messaging in the original code.
>>>  *You'd have to scroll down the entire thing to see that.
>>>  RETURN .F.
>>>
>>>ENDIF
>>>
>>>IF RECCOUNT("FILEC") = 0
>>>  DO XCENTER With "There is no data to Append",15
>>>  READ
>>>  RETURN .F.
>>>ENDIF
>>>
>>>PRIVATE pnSelect, pnDollars, pcAction
>>>m.pnSelect = SELECT()
>>>m.pcAction = 'A'
>>>
>>>SELECT FILEC
>>>SUM amount TO m.pnDollars
>>>
>>>@ 6,0  CLEAR TO 20,79
>>>
>>>@ 12,10 SAY XASTR(RECCOUNT()) + " Data to Append - " + ;
>>>          "Value $ " + ALLTRIM(TRANSFORM(m.pnDollars,'9,999,999,999.99'))
>>>
>>>@ 13,10 SAY "Append, Delete, or Cancel? (A/D/C) " ;
>>>      GET m.pcAction PICTURE "!" VALID m.pcAction $ "ADC"
>>>
>>>READ
>>>
>>>DO CASE
>>>CASE m.pcAction = 'A'
>>>   DO XCENTER With "Appending DATA",15
>>>
>>>   *xsavedata can do scatter and gather without creating
>>>   *variables that can be seen by the rest of this module.
>>>   IF NOT XSAVEDATA()
>>>      DO XCENTER WITH ;
>>>           "Append failed! "+;
>>>           "Deleting all loaded data in file.",15
>>>      DELETE FOR appended = .F.
>>>      PACK
>>>      DO CLEANUP
>>>      RETURN .F.
>>>   ENDIF
>>>
>>>   SELECT FILEA
>>>   ZAP
>>>
>>>   SELECT FILEB
>>>   REPLACE ALL appended WITH .T. FOR appended = .F.
>>>
>>>CASE m.pcAction = 'D'
>>>   DO XCENTER With "Deleting all loaded data",15
>>>   SELECT FILEC
>>>   ZAP
>>>
>>>   SELECT FILEB
>>>   DELETE FOR appended = .F.
>>>   PACK
>>>
>>>CASE m.pcAction = 'C'
>>>   DO XCENTER With "Cancelled - data remains",15
>>>   READ
>>>ENDCASE
>>>DO CLEANUP
>>>
>>>RETURN
>>>
>>>FUNCTION CLEANUP
>>>IF m.pnSelect # 0
>>>  SELECT (m.pnSelect)
>>>ENDIF
>>>RETURN
>>>
>>>
>>>
>>>>Hi Mike,
>>>>
>>>>I'd be *very* interested in seeing some examples of that. Thanks!
>>>>
>>>>Evan
>>>>
>>>>
>>>>>Hi Evan
>>>>>
>>>>>That sounds like a really bad example of someone not following any rules. In fact, I think your example doesn't count against having multiple exit points, because it wasn't an example of doing that properly.
>>>>>
>>>>>I'm working with a piece of code that is actually two copies of an entire application, even though there are common modules between the two apps. They often used the one exit per and it is an example of how hard it is to follow that way.
>>>>>
>>>>>I'll try to show you examples from it.
Evan Pauley, MCP
Positronic Technology Systems LLC
Knoxville, TN

If a vegetarian eats vegetables, what does a humanitarian eat?
Previous
Reply
Map
View

Click here to load this message in the networking platform