HiveBrain v1.2.0
Get Started
← Back to all entries
snippetMinor

Deca dimensional ascending and descending array sort function

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
decadescendingarrayandfunctiondimensionalsortascending

Problem

Some folks told that a two-dimensional array can solve almost all the problems, but let me disagree with that. That's why I made this Deca dimensional array sort function. The array must be symmetrical, meaning that it must contain elements corresponding to all the existing elements in the other dimensions.

I didn't put any error handler, because depending on usage purposes, it can take different shapes.

I tried for several days to find a way to avoid such multiple statements by using an on-the-fly variable generator to replace the index of array in the loop. I didn't succeeded, so the question is: did someone know a way to replace the seqArr(i) = arr(s(0), i, s(2) with: seqArr(i) = something based on the s variable (which already is generating the same result)?

The improvement I need is regarding the length of the code. Actually the code is limited to 10 dimensions because of its length, with a solution regarding repetitive statement in the loop it can be extended to an unlimited number of dimensions.

I also don't want to use Microsoft scripting runtime library on this.

```
Function SortArray(ByRef arr As Variant, ByVal selPoint As Variant, ByRef selDim As Integer, Optional ByRef ascend As Boolean = True) As Variant
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
'Deca Dimensional Ascending and Descending Array Sort Function
'Inputs:
'1) arr = one to 10 dimensional symmetrical array
'2) selPoint = selected point index as string e.g. "arr(1,15,4)" or just "(1,15,4)"
'3) selDim = selected dimension, integer from 1 to 10
'4) ascend = Optional ascending or descending direction (default = ascending)
'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''

Dim seq As Variant, seqArr As Variant, s As Variant
Dim i As Integer, j As Integer, arrDim As Integer

On Error Resume Next
Do
arrDim = arrDim + 1
seq = UBound(arr, arrDim)
Loop Until Err.Number <> 0
arrDim

Solution

Let's start with the obvious. If you're trying to do anything with a 10-d array other than put it in a proper database, you're doing it wrong.

That aside, in no particular order, let's begin:

You have too much energy. Seriously. After the second elseif or so your mind should be going "hmm, this seems inefficient, there must be a simpler way to do this".

There is. It's called a Select...Case Statement: MSDN.

This code:

If arrDim = 1 Then
seqArr(i) = arr(s(0))
ElseIf arrDim = 2 And selDim = 1 Then
seqArr(i) = arr(i, s(1))
ElseIf arrDim = 2 And selDim = 2 Then
seqArr(i) = arr(s(0), i)
....


Becomes:

Select Case arrDim

    Case Is = 1
    seqArr(i) = arr(s(0))

    Case Is = 2
        Select Case selDim

            Case Is = 1
            seqArr(i) = arr(i, s(1))

            Case Is = 2
            seqArr(i) = arr(s(0), i)

        End Select

    Case Is = 3
    ......


Select Case is cleaner than elseif, easier to read, easier to follow, and much easier to Alter.

Much better idea: Scrap it entirely, re-write it as a for...loop. And then refactor that into a function that returns the desired array:

For i = LBound(seq) To UBound(seq)

    seqArr(i) = GenerateSequenceArray(s, i, arrDim, selDim)

Next i

Public Function GenerateSequenceArray(ByRef s as Variant, ByVal i as integer, ByVal arrDim as integer, ByVal selDim as integer) As Variant

    Dim j As Long

    Dim arr As Variant
        arr = Array()
        ReDim arr(1 To arrDim)
            For j = 1 To arrDim
                If j = i Then
                    arr(j) = i
                    Else
                    arr(j) = s(j - 1)
                End If
            Next j

            GenerateSequenceArray = arr

End Function


From 115 lines of code to 12. And now, it's absolutely clear what's going on, and it's incredibly easy to change.

Variable Naming: What is s? What does s tell me about the variable? Is it a variable? Or is it just some random mis-typed letter that got into the compiler because Option Explicit wasn't turned on? Meaningful Names. Always.

Do you know what I thought it was? a counter variable. Why? Because it's almost a universal programming law that single letter variables are counters E.G. dim i as long, j as long, k as long. Sure, it's not as neat and tidy, but neat and tidy is useless if I have no idea what's going on.

The same continues elsewhere. I've got seq, seqArr, s. Now, the first 2 are probably sequences of some kind. And the second is supposed to be an array, but beyond that, I've no idea what they are or what they contain.

What is seq?

ReDim seq(UBound(arr, selDim))
For i = LBound(seq) To UBound(seq)
seq(i) = i
Next i


So seq() just contains ascending numbers for each index in the selected dimension? How is that useful, or even necessary? If/when I need it, I'll just write:

For i = 1 to UBound(arr, selDim)
    
Next i


Which is also much clearer about what's going on.

I'm not even going to try and work out how ParseArray works. It's 200 lines of nested for...loops and elseif statements that's been made to work through sheer stubbornness. I guarantee that it can be reduced to a small function like the one above for seqArr. I leave that as an exercise for you.

As a general rule, if the sub/function you're writing is over 50 lines. You should stop, take a long hard look and just check that you can't simplify / refactor it further.

Code Snippets

If arrDim = 1 Then
seqArr(i) = arr(s(0))
ElseIf arrDim = 2 And selDim = 1 Then
seqArr(i) = arr(i, s(1))
ElseIf arrDim = 2 And selDim = 2 Then
seqArr(i) = arr(s(0), i)
....
Select Case arrDim

    Case Is = 1
    seqArr(i) = arr(s(0))

    Case Is = 2
        Select Case selDim

            Case Is = 1
            seqArr(i) = arr(i, s(1))

            Case Is = 2
            seqArr(i) = arr(s(0), i)

        End Select

    Case Is = 3
    ......
For i = LBound(seq) To UBound(seq)

    seqArr(i) = GenerateSequenceArray(s, i, arrDim, selDim)

Next i

Public Function GenerateSequenceArray(ByRef s as Variant, ByVal i as integer, ByVal arrDim as integer, ByVal selDim as integer) As Variant

    Dim j As Long

    Dim arr As Variant
        arr = Array()
        ReDim arr(1 To arrDim)
            For j = 1 To arrDim
                If j = i Then
                    arr(j) = i
                    Else
                    arr(j) = s(j - 1)
                End If
            Next j

            GenerateSequenceArray = arr

End Function
ReDim seq(UBound(arr, selDim))
For i = LBound(seq) To UBound(seq)
seq(i) = i
Next i
For i = 1 to UBound(arr, selDim)
    <code>
Next i

Context

StackExchange Code Review Q#104630, answer score: 6

Revisions (0)

No revisions yet.