In the previous post I published my version of Funcky aMaxStrLen(). Maurizio commented posting a different version, written using a completely different strategy. I think there is a third way, using FOR EACH. And probably a fourth and a fifth. And probably more.

I put the two versions here, and I will comment on my version, that is correct and wrong at the same time.

// My version
// AMAXSTRLEN: get longest string length in array
FUNCTION aMaxStrLen( a )
   LOCAL n
   LOCAL i
 
   IF Len( a ) == 0
      RETURN 0
   ENDIF
   n := Len( a[ 1 ] )
   FOR i := Len( a ) TO 2 STEP -1
      IF HB_ISCHAR( a[ i ] ) .AND. Len( a[ i ] ) > n
         n := Len( a[ i ] )
      ENDIF
   NEXT
   RETURN n

Now the version posted by Maurizio

// Maurizio version
FUNCTION aMaxStrLen( aArr )
   LOCAL nMaxLen := 0
 
   AEval( aArr, { | e | nMaxLen := Max( nMaxLen, IIf( HB_IsString( e ), Len( e ), 0 ) ) } )
   RETURN nMaxLen

aMaxStrLen() is used just once in the code (actually twice, but in duplicated code), on an array that I’m sure that has all string items: it is created a few lines as an empty string and then populated with aAdd using string values.
So when I started coding the function I was aware of this. And infact I wrote this line of code:

n := Len( a[ 1 ] )

and it is perfectly ok, and is the “correct” part.

The “wrong” part starts just two lines after, when I wrote:

IF HB_ISCHAR( a[ i ] ) .AND. Len( a[ i ] ) > n

Why am I checking if the item is a string if I know that all of them are strings?
So, or I trust the array passed as parameter, or I don’t trust it, and I should check also the first item:

IF HB_ISCHAR( a[ 1 ] )
   n := Len( a[ 1 ] )
ELSE
   n := 0
ENDIF

Of course I won’t write code this way, it is just an example.

Did I commit the over-optimize error? Yes, probably I did, separating the three cases of array lenght: zero lenght, length equal to 1 and length more than 1.
In a real clone of aMaxStrLen, where there may be an array with up to 4096 items (is it the upper limit of Clipper 87 arrays?), it may be nice to have these checks. But this is a special case, where I know that the items won’t be more than a couple dozen.

And now the real, philosophical question: the use of aMaxStrLen in this program is in a special case, within specific bounds. The implementation I did was partly optimized for the specificity of the program. Wouldn’t be better to change the function name to something that makes clear that the recreated function is a version for a specific task?

What do you think: it is better to spend time to write a perfect clone of aMaxStrLen (doing comparisons using tests compiled with Clipper 87 version, since the docs are not very clear on edge cases) or create a specific version giving it a new, derived name? MyAMaxStrLen for example?

How would you write this function?

PS: I’m going to remove this function call. In the loop where the array is populated, I can check the length of the item to be added and keep track of the longest one: no need to duplicate the loop!