A question of proper programming technique

Rees

New Member
Joined
Aug 20, 2003
Messages
16
Can someone help me clarify if this is the best practice, or just extra programming.

To me it seems best to place use a variable to hold the value from a worksheet that is going to be picked up multiple times during run time.

Ie. If I have this code which I created that will tell me the widest column width for a certain column range:

endRange = Range("D6").End(xlToRight).Cells.Column
For x = 4 To endRange
holdWidth = Cells(7, x).ColumnWidth
If holdWidth > largestWidth Then
For y = 4 To endRange
If holdWidth < Cells(7, y).ColumnWidth Then
holdWidth = Cells(7, y).ColumnWidth
largestWidth = Cells(7, y).ColumnWidth
End If
Next y
End If
Next x

I can then use largestWidth for the rest of my procedure


My question is: Do you think it is best practice to assign "Range("D6").End(xlToRight).Cells.Column" to the variable "endRange" and then use that variable in my For loops, or is that just overkill...it seems to me like that should be faster overall.

Any thoughts would be appreciated.

Thanks,

Rees
 

Excel Facts

Select a hidden cell
Somehide hide payroll data in column G? Press F5. Type G1. Enter. Look in formula bar while you arrow down through G.

HalfAce

MrExcel MVP
Joined
Apr 6, 2003
Messages
9,456
Hi Rees,
For what it's worth I believe using endRange would be faster. (Suppose I could be wrong though. Seems to me that actually happened once... :biggrin: )
Dan
 

SIXTH SENSE

Well-known Member
Joined
Oct 29, 2003
Messages
1,883
Rees said:
Can someone help me clarify if this is the best practice, or just extra programming.

To me it seems best to place use a variable to hold the value from a worksheet that is going to be picked up multiple times during run time.

Ie. If I have this code which I created that will tell me the widest column width for a certain column range:

endRange = Range("D6").End(xlToRight).Cells.Column
For x = 4 To endRange
holdWidth = Cells(7, x).ColumnWidth
If holdWidth > largestWidth Then
For y = 4 To endRange
If holdWidth < Cells(7, y).ColumnWidth Then
holdWidth = Cells(7, y).ColumnWidth
largestWidth = Cells(7, y).ColumnWidth
End If
Next y
End If
Next x

I can then use largestWidth for the rest of my procedure


My question is: Do you think it is best practice to assign "Range("D6").End(xlToRight).Cells.Column" to the variable "endRange" and then use that variable in my For loops, or is that just overkill...it seems to me like that should be faster overall.

Any thoughts would be appreciated.

Thanks,

Rees

Hi!
For me, personally, i like that style, to assign that value to
a variable since it will become a "name" to that range. Its much
easier to remember if you name it
Code:
EndOfList1
to reffer to the last cell's row number than having it
Code:
Range("A65536").End(xlup).Row
it makes more sense.
 

dk

MrExcel MVP
Joined
Feb 15, 2002
Messages
2,942
Hi,

If you're going to be referring to property values multiple times then assigning them to a variable can have more than one advantage:-

  1. Your code will become smaller and easier to read.
  2. The code will often run more quickly

To demonstrate my second point try running these two pieces of code:-

Code:
Sub PropertyValuesNotCached()
    Dim a As Long
    Dim sngTimer As Single, lngLoop As Long

    'Approx 20 seconds on my PC

    sngTimer = Timer

    For lngLoop = 1 To 1000000
        a = Range("D6").End(xlToRight).Cells.Column
    Next lngLoop

    MsgBox Timer - sngTimer

End Sub

Sub PropertyValuesCached()
    Dim a As Long, b As Long
    Dim sngTimer As Single, lngLoop As Long

    'Approx 0.03 seconds on my PC

    b = Range("D6").End(xlToRight).Cells.Column

    sngTimer = Timer

    For lngLoop = 1 To 1000000
        a = b
    Next lngLoop

    MsgBox Timer - sngTimer

End Sub

You should see that caching the propery value first has definite performance advantages. Ken Getz (a VBA demi-god) also reports performance improvements in this article.
 

tusharm

MrExcel MVP
Joined
May 28, 2002
Messages
11,029
I would worry less about efficiency and extra programming and more about getting the logic right and understanding the XL object model better.

I reformatted your code for readability:

Code:
    endRange = Range("D6").End(xlToRight).Cells.Column
    For x = 4 To endRange
        holdWidth = Cells(7, x).ColumnWidth
        If holdWidth > largestWidth Then
            For y = 4 To endRange
                If holdWidth < Cells(7, y).ColumnWidth Then
                    holdWidth = Cells(7, y).ColumnWidth
                    largestWidth = Cells(7, y).ColumnWidth
                    End If
                Next y
            End If
        Next x
One of the two loops is essentially redundant! Just step through the code.
The first time through the X loop, largestWidth is uninitialized. Assuming it is declared as some kind of a number, the default value is zero. So, unless the column is hidden, holdWidth will be greater than largestWidth.

The Y loop goes through the entire list of cells-of-interest and sets both holdWidth and largestWidth to the same value -- the largest column width.

Now, when the code returns to the next iteration of the X loop, holdWidth can never be > largestWidth. So, the entire remainder of the X loop is cycled through without any meaningful processing being done!

Also, you are setting the limit of the check with row 6 and then working with row 7. Until and unless XL allows individual rows in the same column to have different widths, there's no need to do that.

Finally, I would definitely object to the name endRange. To me it implies a range object, but it actually refers to a column number. Use something like EndColID or EndColNbr.

So, as a first pass, the code can be simplified to:

Code:
Sub testit()
    Dim aCol  As Range, largestWidth As Single
    For Each aCol In Range(Range("D6"), _
            Range("d6").End(xlToRight))
        largestWidth = Application.WorksheetFunction.Max( _
            largestWidth, aCol.ColumnWidth)
        Next aCol
    MsgBox largestWidth
    End Sub
This still has two references to Range("D6"). If someone was to ever maintain this code, there is a small probability of introducing an error. The next version has a single reference to the starting point:

Code:
Sub testit2()
    Dim aCol  As Range, largestWidth As Single
    With Range("D6")
    For Each aCol In Range(.Cells(1), .End(xlToRight))
        largestWidth = Application.WorksheetFunction.Max( _
            largestWidth, aCol.ColumnWidth)
        Next aCol
        End With
    MsgBox largestWidth
    End Sub

This is still not very general purpose. All the work and all it does is address a single hardcoded range. Make it a function and its utility will surge:

Code:
Function MaxColWidth(aRng As Range) As Single
    Dim aCol As Range
    For Each aCol In aRng.Columns
        MaxColWidth = Application.WorksheetFunction.Max(MaxColWidth, aCol.ColumnWidth)
        Next aCol
    End Function
Sub testMaxColWidth()
    With Range("d6")
    MsgBox MaxColWidth(Range(.Cells(1, 1), .End(xlToRight)).Offset(1, 0))
        End With
End Sub

I would focus on this kind of 'best practice' first and then worry about other stuff.

Rees said:
Can someone help me clarify if this is the best practice, or just extra programming.

To me it seems best to place use a variable to hold the value from a worksheet that is going to be picked up multiple times during run time.

Ie. If I have this code which I created that will tell me the widest column width for a certain column range:

endRange = Range("D6").End(xlToRight).Cells.Column
For x = 4 To endRange
holdWidth = Cells(7, x).ColumnWidth
If holdWidth > largestWidth Then
For y = 4 To endRange
If holdWidth < Cells(7, y).ColumnWidth Then
holdWidth = Cells(7, y).ColumnWidth
largestWidth = Cells(7, y).ColumnWidth
End If
Next y
End If
Next x

I can then use largestWidth for the rest of my procedure


My question is: Do you think it is best practice to assign "Range("D6").End(xlToRight).Cells.Column" to the variable "endRange" and then use that variable in my For loops, or is that just overkill...it seems to me like that should be faster overall.

Any thoughts would be appreciated.

Thanks,

Rees
 

Rees

New Member
Joined
Aug 20, 2003
Messages
16
Thanks for all the input everyone, and especially, Tushar for taking the time to show me what I could improve on. Thanks again, I am still trying to hack my way through this stuff and appreciate the help that all put in here

Rees
 
Master Excel Bundle

Excel contains over 450 functions, with more added every year. That’s a huge number, so where should you start? Right here with this bundle.

Forum statistics

Threads
1,152,095
Messages
5,768,053
Members
425,451
Latest member
JohnBrooksBiddle

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Top