Skip to main content
Commonmark migration
Source Link

Output sheet

###Output sheet FirstFirst things first - you aren't checking that your output sheet exists and will error if it doesn't.

###Variable names

Variable names

###Base 1

Base 1

Original Data

###Original Data TheThe way you're getting the default data is a little bit sketchy. Also, setting your target book to ThisWorkbook makes it so you need to always use the macro's book. You can't have any error handling when you just make the assumption that your target sheet will be the first sheet.

Populating the first array

###Populating the first array II don't understand the offsetRows argument, so

###GetDate

GetDate

###GetCatagory

GetCatagory

###GetFinalColumn

GetFinalColumn

###Output sheet First things first - you aren't checking that your output sheet exists and will error if it doesn't.

###Variable names

###Base 1

###Original Data The way you're getting the default data is a little bit sketchy. Also, setting your target book to ThisWorkbook makes it so you need to always use the macro's book. You can't have any error handling when you just make the assumption that your target sheet will be the first sheet.

###Populating the first array I don't understand the offsetRows argument, so

###GetDate

###GetCatagory

###GetFinalColumn

Output sheet

First things first - you aren't checking that your output sheet exists and will error if it doesn't.

Variable names

Base 1

Original Data

The way you're getting the default data is a little bit sketchy. Also, setting your target book to ThisWorkbook makes it so you need to always use the macro's book. You can't have any error handling when you just make the assumption that your target sheet will be the first sheet.

Populating the first array

I don't understand the offsetRows argument, so

GetDate

GetCatagory

GetFinalColumn

body breaks
Source Link
Raystafarian
  • 7.3k
  • 1
  • 23
  • 60
 
 
 
 
 
 
 
Dim finalColumn As Long

        For finalColumn = 9 To 12
            finalArray(categoryByRowi, finalColumn) = dataArray(sourceRow, GetFinalColumn(finalColumn, categoryByRow)) 'finalColumn
        Next 

Public Function GetCategory(ByVal categoryByRow As Long) As String
    Select Case categoryByRow
    Case 1
        GetCategory = "Cat1: Life Threatening"
    Case 2
        GetCategory = "Cat2: Emergency"
    Case 3
        GetCategory = "Cat3: Urgent"
    Case 0
        GetCategory = "Cat4: Less Urgent"
    Case Else
        GetCategory = "Unknown"
    End Select
End Function

Public Function GetFinalColumn(ByVal finalColumn As Long, ByVal categoryByRow As Long) As Long
    Select Case categoryByRow
    Case 0
        categoryByRow = 3
    Case 1
        categoryByRow = 0
    Case 2
        categoryByRow = 1
    Case 3
        categoryByRow = 2
    End Select

    Select Case finalColumn
    Case 9
        GetFinalColumn = 6 + categoryByRow
    Case 10
        GetFinalColumn = 10 + categoryByRow
    Case 11
        GetFinalColumn = 14 + categoryByRow
    Case 12
        GetFinalColumn = 18 + categoryByRow
    End Select
End Function
Dim finalColumn As Long

        For finalColumn = 9 To 12
            finalArray(categoryByRow, finalColumn) = dataArray(sourceRow, GetFinalColumn(finalColumn, categoryByRow)) 'finalColumn
        Next 

Public Function GetCategory(ByVal categoryByRow As Long) As String
    Select Case categoryByRow
    Case 1
        GetCategory = "Cat1: Life Threatening"
    Case 2
        GetCategory = "Cat2: Emergency"
    Case 3
        GetCategory = "Cat3: Urgent"
    Case 0
        GetCategory = "Cat4: Less Urgent"
    Case Else
        GetCategory = "Unknown"
    End Select
End Function

Public Function GetFinalColumn(ByVal finalColumn As Long, ByVal categoryByRow As Long) As Long
    Select Case categoryByRow
    Case 0
        categoryByRow = 3
    Case 1
        categoryByRow = 0
    Case 2
        categoryByRow = 1
    Case 3
        categoryByRow = 2
    End Select

    Select Case finalColumn
    Case 9
        GetFinalColumn = 6 + categoryByRow
    Case 10
        GetFinalColumn = 10 + categoryByRow
    Case 11
        GetFinalColumn = 14 + categoryByRow
    Case 12
        GetFinalColumn = 18 + categoryByRow
    End Select
End Function
 
 
 
 
 
 
 
Dim finalColumn As Long

        For finalColumn = 9 To 12
            finalArray(i, finalColumn) = dataArray(sourceRow, GetFinalColumn(finalColumn, categoryByRow)) 'finalColumn
        Next 

Public Function GetCategory(ByVal categoryByRow As Long) As String
    Select Case categoryByRow
    Case 1
        GetCategory = "Cat1: Life Threatening"
    Case 2
        GetCategory = "Cat2: Emergency"
    Case 3
        GetCategory = "Cat3: Urgent"
    Case 0
        GetCategory = "Cat4: Less Urgent"
    Case Else
        GetCategory = "Unknown"
    End Select
End Function

Public Function GetFinalColumn(ByVal finalColumn As Long, ByVal categoryByRow As Long) As Long
    Select Case categoryByRow
    Case 0
        categoryByRow = 3
    Case 1
        categoryByRow = 0
    Case 2
        categoryByRow = 1
    Case 3
        categoryByRow = 2
    End Select

    Select Case finalColumn
    Case 9
        GetFinalColumn = 6 + categoryByRow
    Case 10
        GetFinalColumn = 10 + categoryByRow
    Case 11
        GetFinalColumn = 14 + categoryByRow
    Case 12
        GetFinalColumn = 18 + categoryByRow
    End Select
End Function
added 3337 characters in body
Source Link
Raystafarian
  • 7.3k
  • 1
  • 23
  • 60

Your use of i and j is acceptable, but why not give the reader some context like 'targetFirstDimension`targetFirstDimension etc?

###GetFinalColumn

Hm. How does this work?

For n = 9 To 12
      finalArray(i, n) = dataArray(sourceRow, GetFinalColumn(n, i)) 'finalColumn
Next n

Your variables in the argument and in the function aren't telling me much. It all seems arbitrary at first look. You're basing what column to use on what row is being used, correct? And you do n = 9 to 12 for every i? This seems pretty round-about. You've already done i Mod 4 in GetCategory.

For every 4 rows, change the final column by 4 starting at 6

Right? So the essentially Category determines the column?

Dim sourceRow As Long
Dim categoryByRow As Long

For i = LBound(finalArray, 1) To UBound(finalArray, 1)
    categoryByRow = i Mod 4

You moved the mod up before GetCategory, so you can just pass that and remove the Mod from that function. You can also pass the new variable to GetLastColumn. You also pass n to GetLastColumn, turn it into lastColumn and then create a new n. That's confusing

Dim finalColumn As Long

        For finalColumn = 9 To 12
            finalArray(categoryByRow, finalColumn) = dataArray(sourceRow, GetFinalColumn(finalColumn, categoryByRow)) 'finalColumn
        Next 

Public Function GetCategory(ByVal categoryByRow As Long) As String
    Select Case categoryByRow
    Case 1
        GetCategory = "Cat1: Life Threatening"
    Case 2
        GetCategory = "Cat2: Emergency"
    Case 3
        GetCategory = "Cat3: Urgent"
    Case 0
        GetCategory = "Cat4: Less Urgent"
    Case Else
        GetCategory = "Unknown"
    End Select
End Function

Public Function GetFinalColumn(ByVal finalColumn As Long, ByVal categoryByRow As Long) As Long
    Select Case categoryByRow
    Case 0
        categoryByRow = 3
    Case 1
        categoryByRow = 0
    Case 2
        categoryByRow = 1
    Case 3
        categoryByRow = 2
    End Select

    Select Case finalColumn
    Case 9
        GetFinalColumn = 6 + categoryByRow
    Case 10
        GetFinalColumn = 10 + categoryByRow
    Case 11
        GetFinalColumn = 14 + categoryByRow
    Case 12
        GetFinalColumn = 18 + categoryByRow
    End Select
End Function

Still, this feels pretty clunky. Why does the i mod 4 give a non-result?

categoryByRow = categoryByRow - 1
If categoryByRow < 0 Then categoryByRow = 3

And your finalColumn also give a non-result? I think there must be a way to refactor that, but I can't figure out the relationship of {9-6,10-10,11-14,12-18}. Someone better at math might be able to.

Public Function GetFinalColumn(ByVal finalColumn As Long, ByVal categoryByRow As Long) As Long
    
    categoryByRow = categoryByRow - 1
    If categoryByRow < 0 Then categoryByRow = 3
    
    Select Case finalColumn
    Case 9
        GetFinalColumn = 6 + categoryByRow
    Case 10
        GetFinalColumn = 10 + categoryByRow
    Case 11
        GetFinalColumn = 14 + categoryByRow
    Case 12
        GetFinalColumn = 18 + categoryByRow
    End Select
End Function

Your use of i and j is acceptable, but why not give the reader some context like 'targetFirstDimension` etc?

Your use of i and j is acceptable, but why not give the reader some context like targetFirstDimension etc?

###GetFinalColumn

Hm. How does this work?

For n = 9 To 12
      finalArray(i, n) = dataArray(sourceRow, GetFinalColumn(n, i)) 'finalColumn
Next n

Your variables in the argument and in the function aren't telling me much. It all seems arbitrary at first look. You're basing what column to use on what row is being used, correct? And you do n = 9 to 12 for every i? This seems pretty round-about. You've already done i Mod 4 in GetCategory.

For every 4 rows, change the final column by 4 starting at 6

Right? So the essentially Category determines the column?

Dim sourceRow As Long
Dim categoryByRow As Long

For i = LBound(finalArray, 1) To UBound(finalArray, 1)
    categoryByRow = i Mod 4

You moved the mod up before GetCategory, so you can just pass that and remove the Mod from that function. You can also pass the new variable to GetLastColumn. You also pass n to GetLastColumn, turn it into lastColumn and then create a new n. That's confusing

Dim finalColumn As Long

        For finalColumn = 9 To 12
            finalArray(categoryByRow, finalColumn) = dataArray(sourceRow, GetFinalColumn(finalColumn, categoryByRow)) 'finalColumn
        Next 

Public Function GetCategory(ByVal categoryByRow As Long) As String
    Select Case categoryByRow
    Case 1
        GetCategory = "Cat1: Life Threatening"
    Case 2
        GetCategory = "Cat2: Emergency"
    Case 3
        GetCategory = "Cat3: Urgent"
    Case 0
        GetCategory = "Cat4: Less Urgent"
    Case Else
        GetCategory = "Unknown"
    End Select
End Function

Public Function GetFinalColumn(ByVal finalColumn As Long, ByVal categoryByRow As Long) As Long
    Select Case categoryByRow
    Case 0
        categoryByRow = 3
    Case 1
        categoryByRow = 0
    Case 2
        categoryByRow = 1
    Case 3
        categoryByRow = 2
    End Select

    Select Case finalColumn
    Case 9
        GetFinalColumn = 6 + categoryByRow
    Case 10
        GetFinalColumn = 10 + categoryByRow
    Case 11
        GetFinalColumn = 14 + categoryByRow
    Case 12
        GetFinalColumn = 18 + categoryByRow
    End Select
End Function

Still, this feels pretty clunky. Why does the i mod 4 give a non-result?

categoryByRow = categoryByRow - 1
If categoryByRow < 0 Then categoryByRow = 3

And your finalColumn also give a non-result? I think there must be a way to refactor that, but I can't figure out the relationship of {9-6,10-10,11-14,12-18}. Someone better at math might be able to.

Public Function GetFinalColumn(ByVal finalColumn As Long, ByVal categoryByRow As Long) As Long
    
    categoryByRow = categoryByRow - 1
    If categoryByRow < 0 Then categoryByRow = 3
    
    Select Case finalColumn
    Case 9
        GetFinalColumn = 6 + categoryByRow
    Case 10
        GetFinalColumn = 10 + categoryByRow
    Case 11
        GetFinalColumn = 14 + categoryByRow
    Case 12
        GetFinalColumn = 18 + categoryByRow
    End Select
End Function
added 561 characters in body
Source Link
Raystafarian
  • 7.3k
  • 1
  • 23
  • 60
Loading
added 561 characters in body
Source Link
Raystafarian
  • 7.3k
  • 1
  • 23
  • 60
Loading
added 561 characters in body
Source Link
Raystafarian
  • 7.3k
  • 1
  • 23
  • 60
Loading
added 1079 characters in body
Source Link
Raystafarian
  • 7.3k
  • 1
  • 23
  • 60
Loading
added 1079 characters in body
Source Link
Raystafarian
  • 7.3k
  • 1
  • 23
  • 60
Loading
added 1079 characters in body
Source Link
Raystafarian
  • 7.3k
  • 1
  • 23
  • 60
Loading
added 1079 characters in body
Source Link
Raystafarian
  • 7.3k
  • 1
  • 23
  • 60
Loading
Source Link
Raystafarian
  • 7.3k
  • 1
  • 23
  • 60
Loading