_ArraySwap problems

[Quirinus]

1. The $bRow default value in the 3.3.13.15 documentation is listed as True. In 3.3.12.0 it was listed as False. In both cases it's False in the Array.au3. So, for the new version, it should be changed to $bRow = True in the Array.au3.

2. _ArraySwap isn't listed in the internal Array.au3 function list.

3. There's no check for $iIndex_2 bounds.

Just double these two lines:

If $iIndex_1 < 0 Or $iIndex_1 > $iDim_2 Then Return SetError(4, 0, -1)

If $iIndex_1 < 0 Or $iIndex_1 > $iDim_1 Then Return SetError(4, 0, -1) 

and change $iIndex_1 to $iIndex_2 in the two new lines.

4. I think there's something wrong with $iStart and $iEnd bounds checking. I think this fixes it... switch these two lines:

If $iStart > $iDim_2 Or $iEnd > $iDim_2 Then Return SetError(4, 0, -1)

If $iStart > $iDim_1 Or $iEnd > $iDim_1 Then Return SetError(4, 0, -1)

5. Is it just me, or are the definitions of columns and rows swapped in this function? I might be imagining things because it's late... if it's true, then you might consider renaming $bRow to $bColumn or switching the if-else parts of the code (there's 2 of them).

6. I suggest using -1 as default $iStart and $iEnd value instead of 0, because someone might want to swap just the first (0-th) elements, and this doesn't let him do it (if swaps the whole row/column in that case). Obviously then the bounds checking would have to be written a bit differently to acknowledge this special case. I think it should be done, because the function as is now does not do what is advertised in the documentation.

---

Here's some code that reproduces some of the problems:

#include <Array.au3>

$string = "Id|hcIdx|BaseId|NextInClass|TransLvl|NameStr|MonStatsEx|MonProp" & @CRLF & "skeleton1|0|skeleton1|skeleton2|3|RathmaPriest|zakarumcleric2|other" & @CRLF & "skeleton2|1|skeleton1|skeleton3|1|QuotedForTruth|skeleton2|" & @CRLF & "skeleton3|2|skeleton1|skeleton4|2|BoneWarrior|skeleton3|"
local $array[1][8]
_ArrayAdd($array, $string)
_ArrayDelete($array, 0)
_ArrayDisplay($array)

_ArraySwap($array, 0, 2,True,0,1)
_ArrayDisplay($array)

I haven't checked if any other function has similar problems... I might be wrong in some of these points, so please do point out if I've made a mistake, since it's quite late and I'm tired and sleepy.

I'm checking here before I submit a ticket, if it's needed.

Thanks!

Edited August 6, 2014 by Quirinus

[Melba23]

Quirinus,

I see the problems and I am working on a fix. But a final solution will probably have to wait until the weekend as I am a bit "up in the air" until then.

M23

[Quirinus]

No problem, take it slow, I'm just glad to be of help, and that I've solved the problem (at least from what I can see).

Cheers!

Edited August 7, 2014 by Quirinus

[Melba23]

Quirinus,

Back from the "digital black hole" that is Cornwall.

As you like testing (where were you when I was asking for Beta testers?) here is a modified _ArraySwap function that I think now does what I always thought it did:

expandcollapse popup

#include <Array.au3>

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

_ArrayDisplay($aArray, "Original", Default, 8)
_ArraySwap_Mod($aArray, 3, 7)
ConsoleWrite(@error & @CRLF)
_ArrayDisplay($aArray, "Swapped 3-7", Default, 8)

Local $aArray_Base[10][10]
For $i = 0 To 9
    For $j = 0 To 9
        $aArray_Base[$i][$j] = $i & " - " & $j
    Next
Next
_ArrayDisplay($aArray_Base, "Original", Default, 8)

$aArray = $aArray_Base
_ArraySwap_Mod($aArray, 3, 7)
ConsoleWrite(@error & @CRLF)
_ArrayDisplay($aArray, "Swapped Rows 3-7", Default, 8)

$aArray = $aArray_Base
_ArraySwap_Mod($aArray, 3, 7, False, 2, 5)
ConsoleWrite(@error & @CRLF)
_ArrayDisplay($aArray, "Swapped Rows 3-7, Cols 2-5", Default, 8)

$aArray = $aArray_Base
_ArraySwap_Mod($aArray, 3, 7, True)
ConsoleWrite(@error & @CRLF)
_ArrayDisplay($aArray, "Swapped Cols 3-7", Default, 8)

$aArray = $aArray_Base
_ArraySwap_Mod($aArray, 3, 7, True, 2, 5)
ConsoleWrite(@error & @CRLF)
_ArrayDisplay($aArray, "Swapped Cols 3-7, Rows 2-5", Default, 8)

Func _ArraySwap_Mod(ByRef $avArray, $iIndex_1, $iIndex_2, $bCol = False, $iStart = -1, $iEnd = -1)

    If $bCol = Default Then $bCol = False
    If $iStart = Default Then $iStart = -1
    If $iEnd = Default Then $iEnd = -1
    If Not IsArray($avArray) Then Return SetError(1, 0, -1)
    Local $iDim_1 = UBound($avArray, $UBOUND_ROWS) - 1
    Local $iDim_2 = UBound($avArray, $UBOUND_COLUMNS) - 1
    If $iDim_2 = -1 Then ; 1D array so force defaults
        $bCol = False
        $iStart = -1
        $iEnd = -1
    EndIf
    ; Bounds check
    If $iStart > $iEnd Then Return SetError(5, 0, -1)
    If $bCol Then
        If $iIndex_1 < 0 Or $iIndex_2 > $iDim_2 Then Return SetError(3, 0, -1)
        If $iStart = -1 Then $iStart = 0
        If $iEnd = -1 Then $iEnd = $iDim_1
    Else
        If $iIndex_1 < 0 Or $iIndex_2 > $iDim_1 Then Return SetError(3, 0, -1)
        If $iStart = -1 Then $iStart = 0
        If $iEnd = -1 Then $iEnd = $iDim_2
    EndIf
    Local $vTmp
    Switch UBound($avArray, $UBOUND_DIMENSIONS)
        Case 1
            $vTmp = $avArray[$iIndex_1]
            $avArray[$iIndex_1] = $avArray[$iIndex_2]
            $avArray[$iIndex_2] = $vTmp
        Case 2
            If $iStart < -1 Or $iEnd < -1 Then Return SetError(4, 0, -1)
            If $bCol Then
                If $iStart > $iDim_1 Or $iEnd > $iDim_1 Then Return SetError(4, 0, -1)
                For $j = $iStart To $iEnd
                    $vTmp = $avArray[$j][$iIndex_1]
                    $avArray[$j][$iIndex_1] = $avArray[$j][$iIndex_2]
                    $avArray[$j][$iIndex_2] = $vTmp
                Next
                Else
                If $iStart > $iDim_2 Or $iEnd > $iDim_2 Then Return SetError(4, 0, -1)
                For $j = $iStart To $iEnd
                    $vTmp = $avArray[$iIndex_1][$j]
                    $avArray[$iIndex_1][$j] = $avArray[$iIndex_2][$j]
                    $avArray[$iIndex_2][$j] = $vTmp
                Next
            EndIf
        Case Else
            Return SetError(2, 0, -1)
    EndSwitch

    Return 1

EndFunc   ;==>_ArraySwap_Mod

Please let me know if it does not meet with your expectations - and comments from others welcome too, of course.

M23

Edited August 15, 2014 by Melba23
New code

[Quirinus]

Nice, I'll check it out tomorrow and let you know!