Plateforme Level Extreme
Abonnement
Profil corporatif
Produits & Services
Support
Légal
English
One exit per procedure/function/codeblock to what purpos
Message
Information générale
Forum:
Visual FoxPro
Catégorie:
Autre
Divers
Thread ID:
00835552
Message ID:
00836625
Vues:
20
Evan,
I think that your modifications do not actually do the same thing as Cetin's code. If !.GetValidDateRanges(), you are setting return value to be .F. yet you continue processing and changing the return value in several places which is not the same thing as what Cetin was trying to accomplish.

I think that the following code does the same thing as Cetin's code allowing only one exit point, but IMHO, I think that Cetin's code is more readable. I had to add multiple comments to follow the logic and separated the If !.GetValidDateRanges() and its ENDIF by many more lines when it seems clear that the purpose was to early exit without processing anymore if it failed this condition. I even added end of line comments with the endif's to help follow the logic to know which was the close structure for which IF condition since it was so far away for the beginning of the conditional structure. Some of my added comments are exagerated and overkill, but done to show the original intent of Cetin's example code.
Procedure someproc
  Local luReturnValue, ...
  luReturnValue = .f.
With This
    If !.GetValidDateRanges()
      *!* don't need to continue since !.GetValidDateRanges() returned .f.
    else 
      *!* only continue if .GetValidDateRanges() returns .t.
      * Some other code ...
      Select ... From ...Into Array .arrItems
      If Type('.arrItems[1,1]')#'N'
        *!* don't need to continue since Type('.arrItems[1,1]')#'N'
      else
        *!* only continue if Type('.arrItems[1,1]')='N'
        For ix=1 To Alen(.arrItems,1)
          If .arrItems[ix,2] = 'somecondition'
            luReturnValue = .SomeOtherProc(ix)
            *!* this assumes that .SomeOtherProc(ix) returns a logical, otherise
            *!* this procedure will return different data types which will bite you in the rear
            *!* I have my return value so I am done with this procedure as Cetin exited it here
             exit
          Endif
          If .arrItems[ix,3] = 'somecondition2'
            luReturnValue = .SomeOtherProc2(ix)
            *!* this assumes that .SomeOtherProc(ix) returns a logical, otherise
            *!* this procedure will return different data types which will bite you in the rear
            *!* I have my return value so I am done with this procedure as Cetin exited it here
          exit
          Endif
        Endfor
        if luReturnValue
          *!* only continue processing if everything up to this point is correct

          * Some other code where also there are few other returns ...

        endif
      Endif && Type('.arrItems[1,1]')='N'
    Endif && .GetValidDateRanges()
  Endwith
  RETURN ( luReturnValue )

Endproc
I really miss the beautify option that was available in FPW 2.6 that allowed you to beautify code with "action diagram symbols" as it would transform most any code into a readable, easy to follow format by joining the structures and showing returns with < ==Return. In fact, when I look at unfamilliar code, old legacy code, poorly written code, or have made modifications and inadvertantly have mismatched structures (if/endif, do/enddo,do case/endcase or for/next) I will often open FPW26 and paste code into a program file and beautify with action diagram symbols just to quickly get a handle on what is going on. It works for most code except for code structures not supported in FPW26. The following is Cetin's code beautified in FPW2.6 with Action diagram symbols (except the I had to replace the graphics characters would be nice neat lines and arrows with characters that would print here). IMO, The ability to diagram code like this makes almost any code more readable and easier follow the intent and logic.
     Procedure someproc
        Local ...
        With This
      |- If !.GetValidDateRanges()
<=====|=====Return .F.
      |-Endif
        * Some other code ...
        Select ... From ...Into Array .arrItems
      |-If Type('.arrItems[1,1]')#'N'
<=====|=====Return .F.  && No items
      |-Endif
      
      ||=For ix=1 To Alen(.arrItems,1)
      ||
      ||  |-If .arrItems[ix,2] = 'somecondition'
<=====||==|====Return .SomeOtherProc(ix)
      ||  |-Endif
      ||
      ||  |-If .arrItems[ix,3] = 'somecondition2'
<=====||==|=====Return .SomeOtherProc2(ix)
      ||  |-Endif
      ||
      ||-Endfor
        * Some other code where also there are few other returns ...
        Endwith
        Endproc
>Hi Cetin,
>
>I'm sorry, I don't agree with your assertion ** IN THIS INSTANCE **.
>
>While your code is readable and operates flawlessly, and I would NEVER assert that this was "bad" or "incorrect" code, I would structure it as follows:
>
>
>Procedure someproc
>*-- Evan would replace
>* Local ...
>*-- with
>Local luReturnValue, ...
>With This
>  If !.GetValidDateRanges()
>    *-- Evan would replace
>    * Return .F.
>    *-- with
>    luReturnValue = .F.
>  Endif
>  * Some other code ...
>  Select ... From ...Into Array .arrItems
>  If Type('.arrItems[1,1]')#'N'
>    *-- Evan would replace
>    * Return .F.  && No items
>    *-- with
>    luReturnValue = .F.
>  Endif
>  For ix=1 To Alen(.arrItems,1)
>    If .arrItems[ix,2] = 'somecondition'
>      *-- Evan would replace
>      * Return .SomeOtherProc(ix)
>      *-- with
>      luReturnValue = .SomeOtherProc(ix)
>    Endif
>    If .arrItems[ix,3] = 'somecondition2'
>      *-- Evan would replace
>      * Return .SomeOtherProc2(ix)
>      *-- with
>      luReturnValue = .SomeOtherProc2(ix)
>    Endif
>  Endfor
>  * Some other code where also there are few other returns ...
>Endwith
>*-- Evan would add here:
>RETURN ( luReturnValue )
>
>Endproc
>
>
>This accomplishes EXACTLY the same end result with a SINGLE EXIT POINT, and does NOT slow the code down or make the code ANY LESS READABLE. It also does not restrict breaking the code down into "handler routines" as you suggest (which is what I also do -- great minds -g-).
>
>Please, Cetin, I'm NOT saying I'm right and you're wrong, or even hinting that your code is anything less than excellent -- this is a *personal style* issue, and (as you correctly suggest) one we could debate forever without settling anything (g). My point is that IN THE INSTANCE YOU PROVIDE, I don't believe that multiple exit points are more efficient.
>
>Evan
>
>
>>Hi Mike,
>>This is kind of issue that could be argued for a decade :)
>>I don't think only the ones at the start are fine. Anywhere appearing a return might be fine as well. ie: Just at the moment there is some code I'm working on and that has a 'return .f.' in about the middle of the routine. I reread that code carefully and glad that I placed that there from the start :)
>>Preventing 'abuse' of many returns might cause abuse of creating unnecessary variables/objects :)
>>It's hard to decide how much is too many as it's hard to show a sample code that proves one is superior to other. For example if I pseudocode the current routine here :
>>
>>
>>Procedure someproc
>>Local ...
>>With This
>>  If !.GetValidDateRanges()
>>    Return .F.
>>  Endif
>>  * Some other code ...
>>  Select ... From ...Into Array .arrItems
>>  If Type('.arrItems[1,1]')#'N'
>>    Return .F.  && No items
>>  Endif
>>  For ix=1 To Alen(.arrItems,1)
>>    If .arrItems[ix,2] = 'somecondition'
>>      Return .SomeOtherProc(ix)
>>    Endif
>>    If .arrItems[ix,3] = 'somecondition2'
>>      Return .SomeOtherProc2(ix)
>>    Endif
>>  Endfor
>>  * Some other code where also there are few other returns ...
>>Endwith
>>Endproc
>>
>>This already has the code broken down into many handler routines. From my POV it's easier to follow this code then to follow 'one exit point' counterpart.
>>Cetin
>>
>>
>>>Hi Cetin
>>>
>>>It seems the issue here is having too many returns. The ones at the start that jump out when the most obvious stuff fails is fine.
>>>
>>>IMO the abuse of many returns will occur in the "meat" of the routine. This may be because there are too many kinds of meat in routines. Maybe if there are too many returns, that can be taken as indication that the routine should be broken down?
>>>
Précédent
Suivant
Répondre
Fil
Voir

Click here to load this message in the networking platform