Hi Cetin,
I disagree, Your code was not really hard to follow, expecially with the ability to diagram it with "action diagram symbols", a beautify feature long since lost :-( . My logic error was caused by my attempt to modify a short simple routine that worked correctly and efficiently, just for the purpose of a single exit point (and to illustrate that Evan had NOT correctly duplicated the logic) and I neglected to include a formerly unnecessary llContinue variable.
I use a single exit point when it makes sense, and muliple exit points where it makes sense. I really hate having to define unnecessary variables and unneeded conditional logic, or splitting code that is tightly related and not really reuseable into multiple functions and sub methods due to someone's "RULES" for one exit point or just to keep each progam file less than one page. I consider it a real PITA and more of a maintenance headache when I have to read through a program and have to open many other subprograms to see what they do and why their return value is not what I expected just to get to the meat of the code. If subfunctions were hyperlinked when reading code, it might make it more readable, but switching between 50 short non reuseable functions just to get to the meat of a procedure is maddening. I prefer to have one long program with clearly defined inputs, ample comments and a clearly defined purpose than numerous nonreuseable code snippets which can be like having a bolt and each time you look at it, you have to locate the matching nut from a bucket of nuts of every size, when if you had stored the bolt with the nut attached, it would be right there without the extra effort.
One thing that I often do is to have the return value of a function as numeric where Less than or equal to 0 is a failing result and anything greater than 0 is a successful result. This way if I have multiple reasons for it to fail or succeed or if it has multiple conditional branches, I can check the return value and quickly determine what it did and how the result was obtained and which branch or condition executed. I use this style with single exit point or multiple exit point code and I have found that it helps with debugging to know which condition set the return result, no matter how many exit points I used.
Just my .02 cents
Elmer
>Elmer,
>A very close catch :) Reading the modification codes on my code I've to agree at least my code was hard to follow for someone else to take over :)
>One hidden implication is that with my style you (at least I do myself) see the return value type is a logical and other procedures are returning logicals too.
>Your code got a close match with the exception :
>
>> if luReturnValue
>> *!* only continue processing if everything up to this point is correct
>
>luReturnValue being .t. or .f. doesn't imply the code to continue or stop. If one of the procedures in loop is called once than luReturnValue should be its return value and code musn't process any further lines down to final return (with one exit point). Another llContinue variable :(
>Cetin
>
>>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
>> Endif
>> 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.
>> |-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.
>>> *-- 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.
>>>> 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?
>>>>>
