CODEWITHSUNDEEP
excelvba
methuselah
methuselah

Reputation: 13206

Quicker way of writing for each statement in VBA

I'm writing VBA code to fill three different combo-boxes with the same data. I was just wondering if there is a more efficient way of writing it then what I am doing at the moment?

' Create fac1 cbo
For Each c_fac In ws_misc.Range("fac")
    With Me.cbo_fac1
    .AddItem c_fac.Value
    .List(.ListCount - 1, 1) = c_fac.Offset(0, 1).Value
    End With
Next c_fac

' Create fac2 cbo
For Each c_fac In ws_misc.Range("fac")
    With Me.cbo_fac2
    .AddItem c_fac.Value
    .List(.ListCount - 1, 1) = c_fac.Offset(0, 1).Value
    End With
Next c_fac

' Create fac3 cbo
For Each c_fac In ws_misc.Range("fac")
    With Me.cbo_fac3
    .AddItem c_fac.Value
    .List(.ListCount - 1, 1) = c_fac.Offset(0, 1).Value
    End With
Next c_fac

Thanks for taking the time!

Upvotes: 4

Views: 633

Answers (4)

SWa
SWa

Reputation: 4363

This would be much faster (on the assumption that "fac" is a column), .AddItem is very slow for > 5 entries:

Dim rng
rng = ws_misc.Range("fac").resize(,2).value
Me.cbo_fac1.List = rng
Me.cbo_fac2.List = rng
Me.cbo_fac3.List = rng

Upvotes: 2

nutsch
nutsch

Reputation: 5962

One step further, possibly:

dim lLoop as long
' Create fac1 cbo
For Each c_fac In ws_misc.Range("fac")
    For lLoop=1 to 3
        Me.controls("cbo_fac" & lLoop).AddItem c_fac.Value
        Me.controls("cbo_fac" & lLoop).List(Me.controls("cbo_fac" & lLoop).ListCount - 1, 1) = c_fac.Offset(0, 1).Value
    next lLoop
Next c_fac

Upvotes: 8

Cᴏʀʏ
Cᴏʀʏ

Reputation: 107536

Personally I dislike With in this case. It doubles the number of lines of code with next to no benefit. You could shrink this all the way down to:

' Create fac1 cbo
For Each c_fac In ws_misc.Range("fac")

    Me.cbo_fac1.AddItem c_fac.Value
    Me.cbo_fac1.List(Me.cbo_fac1.ListCount - 1, 1) = c_fac.Offset(0, 1).Value

    Me.cbo_fac2.AddItem c_fac.Value
    Me.cbo_fac2.List(Me.cbo_fac2.ListCount - 1, 1) = c_fac.Offset(0, 1).Value

    Me.cbo_fac3.AddItem c_fac.Value
    Me.cbo_fac3.List(Me.cbo_fac3.ListCount - 1, 1) = c_fac.Offset(0, 1).Value

Next c_fac

Upvotes: 1

danielpiestrak
danielpiestrak

Reputation: 5439

Why can't you do this? :

' Create fac1 cbo
For Each c_fac In ws_misc.Range("fac")
    With Me.cbo_fac1
        .AddItem c_fac.Value
        .List(.ListCount - 1, 1) = c_fac.Offset(0, 1).Value
    End With
    With Me.cbo_fac2
        .AddItem c_fac.Value
        .List(.ListCount - 1, 1) = c_fac.Offset(0, 1).Value
    End With
    With Me.cbo_fac3
        .AddItem c_fac.Value
        .List(.ListCount - 1, 1) = c_fac.Offset(0, 1).Value
    End With
Next c_fac

This reduces the number of time syou need to loop through the worksheet range by 2/3rds. Usually reading from and writing to the actual Excel worksheet objects are what takes the most time in Excel VBA code.

Upvotes: 3

Related Questions