Bug in _ArraySearch UDF – Error 4 never triggered

[Norm73]

Hello everyone,

I've found a bug in the _ArraySearch function that contradicts the documentation.

According to the help file, SetError(4) should be returned when $iEnd < $iStart.
However, if $iEnd is out of bounds (e.g., negative), it gets automatically set to the array's upper limit. This means the condition $iStart > $iEnd is never true, and error 4 is never triggered.

Example:

Local $aArray[10] = [1,2,3,4,5,6,7,8,9,10]
_ArraySearch($aArray, 5, 0, -1)

Here, $iEnd = -1 is less than $iStart = 0, so error 4 should occur.
But $iEnd is internally set to UBound($aArray) - 1, which is 9, and the search proceeds normally.

I believe the error check should happen before $iEnd is corrected, or the help file should be updated to reflect the actual behavior.

Can anyone confirm this or has it already been reported?

Best regards

[Nine]

I do not think it would be considered as a bug.  In the UDF it explicitly checks this :

If $iEnd < 1 Or $iEnd > $iDim_2 Then $iEnd = $iDim_2

To get the @error = 4, help file is clear : 4 - $iStart is greater than $iEnd.

So this would generate @error 4 :

_ArraySearch($aArray, 5, 10, -1)

 

[MattyD]

Yeah, I suspect its a cheap way to allow -1 to represent the "default" value, as it does in many other functions.

Just my 2 cents, I wouldn't get too hung up on it.  I can understand the case for error=4 here, but In no world is a negative a valid index, so IMO its reasonable to have programmer take some responsibility. Same argument as "why doesn't it give an error for decimals?"

[Norm73]

Exactly, that's my point:
If the help file states that error 4 should occur when $iEnd < $iStart, then $iEnd must not be modified before that check.
Otherwise, the error check becomes meaningless and the behavior doesn't match the documentation.

My suggestion performs the logical check before any automatic correction — which is more consistent and helps avoid hard-to-trace bugs, like in my case where my script got stuck in a While loop and I couldn't figure out why for quite a while.

I'm not insisting this is a critical bug — but for me, it was much easier to slightly adjust the UDF than to redesign the logic of my own function.

What would speak against handling it like this?
 

; Bounds checking
    If $iStart > $iEnd Then Return SetError(4, 0, -1)
    If $bRow Then
        If UBound($aArray, $UBOUND_DIMENSIONS) = 1 Then Return SetError(5, 0, -1)
        If $iEnd < 1 Or $iEnd > $iDim_2 Then $iEnd = $iDim_2
        If $iStart < 0 Then $iStart = 0
    Else
        If $iEnd < 1 Or $iEnd > $iDim_1 Then $iEnd = $iDim_1
        If $iStart < 0 Then $iStart = 0
    EndIf

 

[Nine]

Nah, that would be even worst.  Let say I want to start at position 3 and search until the end.  Based on help file, I would use :

_ArraySearch($aArray, 5, 3, 0)

But on your logic, I would get an error !  

Edited Saturday at 08:18 PM by Nine

[MattyD]

yeah, I'd imagine if you were going to accommodate for this, you'd want to change $iEnd = 0 meaning "end of array" and make people use the Default keyword. It would be a script breaking change though.

And to be consistent you'll also want to check how "0" and "-1" are used throughout other funcs in the UDF... I'd say there'll be quite a bit of "If $iParam < 1" logic!

 But its not us that you need to persuade - so if you feel strongly enough about it, you can always make the suggestion in the bugtrac .

[Norm73]

Thanks a lot for your feedback!

I see now that your arguments make sense. While I initially thought this was a bug, I understand better now why the function behaves this way.
Still, I think the help file could be a bit clearer to avoid misunderstandings like the one I had.

I'll adjust my script accordingly. Thanks again for the helpful discussion!

Just a small addition from my side:

I also slightly modified the UDF to eliminate duplicate lines of code.
I moved the error 4 check to the end of the bounds checking block, so $iStart is validated after $iEnd has been corrected.
It now looks like this:
 

; Bounds checking
    If $bRow Then
        If UBound($aArray, $UBOUND_DIMENSIONS) = 1 Then Return SetError(5, 0, -1)
        If $iEnd < 1 Or $iEnd > $iDim_2 Then $iEnd = $iDim_2
    Else
        If $iEnd < 1 Or $iEnd > $iDim_1 Then $iEnd = $iDim_1
    EndIf
    If $iStart < 0 Then $iStart = 0
    If $iStart > $iEnd Then Return SetError(4, 0, -1)

Edited Sunday at 05:36 AM by Norm73

[Nine]

  Yes, this part could definitely be optimized.