Using the Same Code for different IF Values

[Denny57]

Following on from an earlier equiry I have successfully managed to reduce over 2500 lines of code by about 90%, however, I believe that this can be further reduced by about another 90% as the code is repeated 8 times, once for every value in a ComboBox. The only difference to each "Block" is the target column range. I have 2 similar questions which I hope can be resolved without the need to re-write much of the code.

Here is a sample of the code showing two of the eight CBO Variable Values

Private Sub cmdThunderballCallDetails_Click()
Dim vCols As Variant
Dim lngRowLoop As Long
Dim lngCtrlLoop As Long
Dim tbCounter As Long

    If cboTBallDrawNumber.Value = "Draw 1" Then
    Set ctn = Sheets("Core Ticket Numbers")
     tbCounter = 1
    vCols = Array("B", "C", "D", "E", "F", "G")
    For lngRowLoop = 44 To 56
        For lngCtrlLoop = 0 To UBound(vCols)
        Me.Controls("txtThunderballSelection" & tbCounter).Text = ctn.Range(vCols(lngCtrlLoop) & lngRowLoop).Value
        tbCounter = tbCounter + 1
       Next
    Next
    Set vtd = Sheets("Variable Ticket Details")
    tbCounter = 1
    vCols = Array("C")
    For lngRowLoop = 46 To 58
        For lngCtrlLoop = 0 To UBound(vCols)
        Me.Controls("cboTballMethodLine" & tbCounter).Text = vtd.Range(vCols(lngCtrlLoop) & lngRowLoop).Value
        tbCounter = tbCounter + 1
        Next
    Next
    tbCounter = 1
    vCols = Array("D")
    For lngRowLoop = 46 To 58
        For lngCtrlLoop = 0 To UBound(vCols)
        Me.Controls("cboTballStatusLine" & tbCounter).Text = vtd.Range(vCols(lngCtrlLoop) & lngRowLoop).Value
        tbCounter = tbCounter + 1
        Next
    Next
    End If

    If cboTBallDrawNumber.Value = "Draw 2" Then
    Set ctn = Sheets("Core Ticket Numbers")
    tbCounter = 1
    vCols = Array("H", "I", "J", "K", "L", "M")
    For lngRowLoop = 44 To 56
        For lngCtrlLoop = 0 To UBound(vCols)
        Me.Controls("txtThunderballSelection" & tbCounter).Text = ctn.Range(vCols(lngCtrlLoop) & lngRowLoop).Value
        tbCounter = tbCounter + 1
       Next
    Next
    Set vtd = Sheets("Variable Ticket Details")
      tbCounter = 1
    vCols = Array("E")
    For lngRowLoop = 46 To 58
        For lngCtrlLoop = 0 To UBound(vCols)
        Me.Controls("cboTballMethodLine" & tbCounter).Text = vtd.Range(vCols(lngCtrlLoop) & lngRowLoop).Value
        tbCounter = tbCounter + 1
        Next
    Next
    tbCounter = 1
    vCols = Array("F")
    For lngRowLoop = 46 To 58
        For lngCtrlLoop = 0 To UBound(vCols)
        Me.Controls("cboTballStatusLine" & tbCounter).Text = vtd.Range(vCols(lngCtrlLoop) & lngRowLoop).Value
        tbCounter = tbCounter + 1
        Next
    Next
    End If

Question 1 = Is there a way to merge the 2 items for Set vtd = Sheets("Variable Ticket Details")? I have tried a number of combinations, none of which were acceptable. The only differences are the target ComboBoxes and the source columns in the same Worksheet.

Question 2 = The only differences in each block are the Column Arrays and the data in cboTBallDrawNumber. The Values of this will be "Draw 1", "Draw 2" .....to "Draw 8". I feel sure that VBA must support the ability to include a variable that would look up the corresponding Arrays.

Please note that the row numbers remain the same for all "cboTBallDrawNumber" variables

As my code works and is much smaller than it was then I am happy for this to remain, But from a learning perspective, it would be useful to ask if this is possible before I start trying some ideas for how this might work.

Many thanks

 

[Micron]

If by shortening the code you mean the lines only, a Select Case block is the only thing that comes to mind wrt reducing a bunch of IF blocks. Anything that was constant could come before the Select block but tbcounter is the only candidate I've spied so far. That may not be possible given that we've no idea what's going on in the other 6 IF blocks. Sometimes you can also shorten the line length by assigning objects to variables and dropping .Value since it's the default anyway. But using variables adds lines, so it's a balance between the two and maybe which style you prefer to read/use.

 

[Denny57]

If by shortening the code you mean the lines only, a Select Case block is the only thing that comes to mind wrt reducing a bunch of IF blocks. Anything that was constant could come before the Select block but tbcounter is the only candidate I've spied so far. That may not be possible given that we've no idea what's going on in the other 6 IF blocks. Sometimes you can also shorten the line length by assigning objects to variables and dropping .Value since it's the default anyway. But using variables adds lines, so it's a balance between the two and maybe which style you prefer to read/use.

Maybe the wording I used was unfortunate.
Basically I am repeating the same 30 lines of code 8 times when the only information that changes is the value of the IF statement and the vCol Array values.
When IF = "Draw 1" the vCols = Array("B", "C", "D", "E", "F", "G")
When IF = "Draw 2" the vCols = Array("H", "I", "J", "K", "L", "M")

I have tried using IF / ELSEIF but this did not work. I am hoping there is a way to use something similer to the above which means I can remove 7 groups of the code.

Incidentally, I need to add a similar piece code to collect information from 6 cells in a different worksheet, all on the same 1 row and to populate 6 different TextBoxes with sequential naming

I get a failure at
For lngRowLoop = 46 To 58
For lngCtrlLoop = 0 To UBound(vCols)
as VBA is expecting a range of rows.

How might I code this when there is just a single row?

Many thanks

 

[Micron]

Based on your code, the only thing you seem to be worried about is the Ubound of the array? In that case, this doesn't make sense to me

    vCols = Array("D")
    For lngRowLoop = 46 To 58
        For lngCtrlLoop = 0 To UBound(vCols)

For this particular array the Ubound is 0, so that's a counter from 0 to 0 thus it never runs? Plus if you don't actually use the array elements why bother with the arrays? If you're going to create an array of 6 elements just to get the number 5 why not just write a counter that is from 0 to 5?
Perhaps I'm still missing a bit of the picture. Trying to cook lunch at the same time, so that's not really helping me!

 

[Denny57]

Based on your code, the only thing you seem to be worried about is the Ubound of the array? In that case, this doesn't make sense to me

    vCols = Array("D")
    For lngRowLoop = 46 To 58
        For lngCtrlLoop = 0 To UBound(vCols)

For this particular array the Ubound is 0, so that's a counter from 0 to 0 thus it never runs? Plus if you don't actually use the array elements why bother with the arrays? If you're going to create an array of 6 elements just to get the number 5 why not just write a counter that is from 0 to 5?
Perhaps I'm still missing a bit of the picture. Trying to cook lunch at the same time, so that's not really helping me!

Thanks Will Try this tomorrow.. Happy New Year

 

[Micron]

OK, I do see where you're using the array element and I'm wrong about a 0 to 0 loop. It does run once, thus you probably should ignore the suggestion.

 

[Micron]

Not sure if any of the following answers questions from post 3, but here's my other thoughts:

You're probably not using Option Explicit because I don't see where some variables are declared (e.g. vtd).

do you have 8 sheet variables? If so, why not just one and keep changing its value in the If or Case blocks?

For single element arrays, the array stuff could be eliminated in favour of the values returned? So for the portion that belongs to vCols = Array("C"), keeping in mind that Ubound(array("C") is 0:

    For lngRowLoop = 46 To 58
        Me.Controls("cboTballMethodLine" & tbCounter).Text = vtd.Range("C0") & lngRowLoop
        tbCounter = tbCounter + 1
    Next

Maybe I'm not understanding what all those pieces do or what should be passed to variables in any particular case, but my mindset is, if the array has just 1 element, use C or D or whatever as the value and forget the array and any variables associated with it. That means that when using a For Next loop that is from 0 to 0, just use 0 where you use the variable (e.g. lngCtrlLoop) and forget the loop.

In the end, I don't think your code will run noticeably faster, but it's probably a good learning exercise.
EDIT - I forgot that trying to colour or bold text within code tags injects html tags so I removed the formatting from my posted code since you can't see it anyway.

 

[Denny57]

Not sure if any of the following answers questions from post 3, but here's my other thoughts:

You're probably not using Option Explicit because I don't see where some variables are declared (e.g. vtd).

do you have 8 sheet variables? If so, why not just one and keep changing its value in the If or Case blocks?

For single element arrays, the array stuff could be eliminated in favour of the values returned? So for the portion that belongs to vCols = Array("C"), keeping in mind that Ubound(array("C") is 0:

    For lngRowLoop = 46 To 58
        Me.Controls("cboTballMethodLine" & tbCounter).Text = vtd.Range("C0") & lngRowLoop
        tbCounter = tbCounter + 1
    Next

Maybe I'm not understanding what all those pieces do or what should be passed to variables in any particular case, but my mindset is, if the array has just 1 element, use C or D or whatever as the value and forget the array and any variables associated with it. That means that when using a For Next loop that is from 0 to 0, just use 0 where you use the variable (e.g. lngCtrlLoop) and forget the loop.

In the end, I don't think your code will run noticeably faster, but it's probably a good learning exercise.
EDIT - I forgot that trying to colour or bold text within code tags injects html tags so I removed the formatting from my posted code since you can't see it anyway.

HI
Thank you for taking the time to review the code and providing possible solutions.. All of this is very much a learning curve for me.. I start with an idea and use my existing but basic knowledge of VBA to compile the code. However, this basic knowledge means that the code I create is long and cumbersome. During this project I have already discovered how to reduce the lines of code by over 2000 lines, but still have so many groups of code that are repeated, but with slight changes to source information.

The code I was trying to apply works fine when there are multiple rows and columns , so I tried to apply this when there is only one row or one column, which is where I have misunderstood the loop functionality.

I did not provide the full code but it does have an Option Explicit but again I misunderstood that I do not need to allocate a different Variable for each worksheet.

Finally, I mentioned that there are 8 pieces of code that are repeated for each instance of a possible value in a ComboBox. There are 2 arrays for columns and 4 Ranges of rows, (8 Combinations). What I was trying to understand is if there is a way to have just one instance of the "Calculation code" maybe as a set instruction with a generic name (e,g, "Run Code"). The example syntax in incorrect but hopefully explains what I am trying to achieve.
Example
1st IF Statement
If ComboBox.Value = 1 Then
(the value of) Row Range = 1 To 4
(the value of) Colummn Array = ("A", "B", "C")
(Action) Run Code
2nd IF Statement
If ComboBox.Value = 2 Then
Row Range = 1 To 4
Colummn Array = ("D", "E", "F")
Run Code
3rd IF Statement
etc

Also, I have tried to shorten the Column Array where the columns are consecutive (eg "A":"C") but I cannot get this to work. Is this possible and if so, please can you provide the correct format.

Sorry if this is a bit long winded but I would very grateful if you can help me to better understand the VBA expectations and restrictions for what I am looking to achieve.

I will be looking for an book on advanced VBA to get a better understanding.

Many thanks

 

[Micron]

What about just adding the ABC stuff as another combo column? Then you don't have to test anything at all? If I pick 1 you can retrieve 1 and ABC. If I pick 2, you retrieve DEF or AL or whatever you have defined the associations of number and letters to be.

 

[Denny57]

Not for the first time today my response has been deleted before I could complete the response.

I really appreciate your help with this enquiry however, the use of a second ComboBox has totallly confused me as whilst I know this is possible I cannot understand how the code will understand that the information in column 2 is an Array. There would still need to be code to indicate this. Also this combo box is also used for other modules so I want to avoid possible problems with these.

Additionally this does not resolve the sought solution to merge the 8 groups of code into 1 with multiple IF options. I did try multiple IF / ELSEIF statements before the Loop code, and whilst VBA did not have a problem with the syntax, nothing happened when I ran the module.

I will keep searching for possible solutions but for the present I think I will stay with the 8 groups of code