_ArrayDelete

[jaberwacky]

I'm looking at the functions inside of Array.au3 and I came across something in _ArrayDelete that I had a question about. 

The section for bounds checking will change anything less than zero or greater than the upbound of the array.  Shouldn't the function instead return an error?  If you're passing anything less than zero to this function then you should probably take a step back and evaluate your situation.

Func _ArrayDelete(ByRef $avArray, Const $iElement)
    If Not IsArray($avArray) Then Return SetError(1, 0, False)
  
    Local Const $iUBound = UBound($avArray) - 1
  
    ; Bounds checking
    If $iElement < 0 Then $iElement = 0
    If $iElement > $iUBound Then $iElement = $iUBound

    ; Move items after $iElement up by 1
    Switch UBound($avArray, 0)
        Case 1
            For $i = $iElement To $iUBound - 1
                $avArray[$i] = $avArray[$i + 1]
            Next
      
            ReDim $avArray[$iUBound]
      
        Case 2
            Local Const $iSubMax = UBound($avArray, 2) - 1
      
            For $i = $iElement To $iUBound - 1
                For $j = 0 To $iSubMax
                    $avArray[$i][$j] = $avArray[$i + 1][$j]
                Next
            Next
      
            ReDim $avArray[$iUBound][$iSubMax + 1]
      
        Case Else
            Return SetError(2, 0, False)
    EndSwitch

    Return $iUBound
EndFunc   ;==>_ArrayDelete

[JohnOne]

If less than 0 is passed to the $iElement parameter then a mistake is assumed and 0 forced.

At that point there has definitely been an array passed anyway.

If I recall I think an AutoIt array actually has elements below zero albeit only used internally, so you don't want them deleted.

[jaberwacky]

I dunno, still seems weird to make that assumption.  What's this about arrays having a negative bound?

I think it still stands that if you're passing a value that is out of bounds of the array then you should reevaluate.

[JohnOne]

It's a valid point you're making without a doubt.

But The UDF wants to only be sure itself does not error out, and puts the onus on the caller to check its arguments.

Edited December 29, 2013 by JohnOne

[Melba23]

jaberwocky6669,

I think JohnOne has put it very nicely. Why depend on a UDF to detect your coding error? You have to check for an error in either case, so why not before rather than after?

Furthermore, trying to cater for every possible parameter error for every UDF would bloat the include files enormously - and given the ingenuity of coders, would probably never catch every single one. In this case setting the parameter to the actual limits of the array seems not unreasonable as it prevents the dreaded "out of bounds" hard crash.

M23

[jaberwacky]

My proposal is to change it to this:

; Bounds checking
If $iElement < 0 Then Return SetError(2, 0, False)
If $iElement > $iUBound Then SetError(3, 0, False)

Which doesn't even attempt to catch every possible error.  Returning an error is less reasonable than an assumption about what the programmer meant?  The UDF also returns errors for other things too like passing non arrays where arrays are expected.

Edited December 29, 2013 by jaberwocky6669

[trancexx]

^^That would be correct.

[Melba23]

jaberwocky6669,

I suggest you open a "Feature Request" in Trac with your suggested solution.

M23

[jchd]

Looks to me like 60% bug and 40% feature request, but anyway it's well worth posting in trac.

[guinness]

Looks to me like 60% bug and 40% feature request, but anyway it's well worth posting in trac.

Correct. If the user is silly enough to pass values outside of the array's range then it's their own fault, but we shouldn't be deleting the 0th or UBound()th entry for their *insert-word-here*, though it's just deserts!

[jaberwacky]

One last question before I do that.  Can someone confirm if this is an off by one error?

; Bounds checking
If $iElement < 0 Then Return SetError(2, 0, False)
If $iElement > $iUBound Then SetError(3, 0, False) ; if there are 10 elements then this only accounts for 11 and higher.

; Bounds checking
If $iElement < 0 Then Return SetError(2, 0, False)
If $iElement >= $iUBound Then SetError(3, 0, False)

[guinness]

First one is appropriate.

[jaberwacky]

So for an 10 element array whose highest index is 9 then if you pass 10 this error check will catch that?

[Mat]

Look at the code again. $iUBound = UBound(...) - 1

[jaberwacky]

KK, thanks for that. 

[guinness]

UBound() dimension parameter could do with a little tweaking by the looks of things. No ticket required as I am on the case.

[guinness]

I have added it for a release with v3.3.11.0. I am torn between adding it to the "script breaking changes" as those that use this function are less likely to error check their code. I will add it.

I don't use the Array UDF, so it makes no odds to me.

Edited December 30, 2013 by guinness