Link to home
Start Free TrialLog in
Avatar of Saqib Husain
Saqib HusainFlag for Pakistan

asked on

Improving efficiency of VBA search in worksheet_change event

I have a part of a worksheet as laid out in the snippet below.

The workbook names and worksheet names are selectable through data validation.

The data validation is updated whenever the user selects the appropriate cell.

Part of the code used to update the validation is given below.

The problem is that I have used the .find method to locate the cells containing the workbook and worksheet names. This is because both the location and the order of files may vary. And every time the selection is changed the find sequence is executed which slows down cursor movement in the worksheet.

Is there any way to do the .find at workbook_open and remember the settings throughout the session?







Private Sub Worksheet_SelectionChange(ByVal Target As Range)
If Target.Rows.Count > 1 Or Target.Columns.Count > 1 Then Exit Sub
wbcol = UsedRange.Find("Workbook").Column
wscol = UsedRange.Find("Worksheet").Column
nslrow = UsedRange.Find("NSL File").Row
desrow = UsedRange.Find("Design File").Row
tplrow = UsedRange.Find("Template File").Row
parrow = UsedRange.Find("Parameters File").Row
prtrow = UsedRange.Find("Printer Name").Row
optrow = UsedRange.Find("Output paramaters").Row
outrow = UsedRange.Find("Output table file").Row
If Target.Column = wbcol Then
    If Target.Row = nslrow Or _
        Target.Row = desrow Or _
        Target.Row = tplrow Or _
        Target.Row = parrow Or _
        Target.Row = optrow Or _
        Target.Row = outrow Then
        wblist = ""
        For Each wb In Application.Workbooks
        wblist = wblist & wb.Name & ","
        'wblist = wblist & wb.Path & "\" & wb.Name & ","
        Next wb
        wblist = Left(wblist, Len(wblist) - 1)
        With Target.Validation
            .Delete
            .Add Type:=xlValidateList, Formula1:=wblist
        End With
        Exit Sub
    End If
End If

If Target.Column = wscol Then
    If Target.Row = nslrow Or _
        Target.Row = desrow Or _
        Target.Row = tplrow Or _
        Target.Row = optrow Or _
        Target.Row = parrow Then
        If Target.Offset(0, wbcol - wscol) = "" Then Exit Sub
        wslist = ""
        For i = 1 To Application.Workbooks.Count
            If Workbooks(i).Name = Target.Offset(0, wbcol - wscol).Value Then w = i: Exit For
        Next i
   
        For Each ws In Application.Workbooks(w).Worksheets
            Select Case Target.Row
                Case optrow
                    If Right(ws.Name, 4) = ".out" Then wslist = wslist & ws.Name & ","
                Case parrow
                    If Right(ws.Name, 4) = ".par" Then wslist = wslist & ws.Name & ","
                Case tplrow
                    If Right(ws.Name, 4) = ".tem" Then wslist = wslist & ws.Name & ","
                Case Else
                    wslist = wslist & ws.Name & ","
            End Select
        Next ws
        wslist = Left(wslist, Len(wslist) - 1)
        With Target.Validation
            .Delete
            .Add Type:=xlValidateList, Formula1:=wslist
        End With
        Exit Sub
    End If
End If
End Sub

Workbook              Worksheet
Design file         c:\prj\dgn\ch1.xls    1L2R
NSL file            c:\prj\data\ch1.xls   1L2R
Parameters file     c:\prog\bin\ch1.xls   lined.par
etc

Open in new window

Avatar of Saqib Husain
Saqib Husain
Flag of Pakistan image

ASKER

This is not how I laid out the code snippet. The header "Workbook" goes over the column with the path name and file name. The header "Worksheet" goes over the 1L2R column. The first column is without a header.

Also, I do not want to write these column and row addresses to any part of the worksheet. I would like VBA to remember the settings, if possible.
Sub Workbook_Open()
public wbcol as range
public wscol as range
public nslrow as range
public desrow as range
public tplrow as range
public parrow as range
public prtrow as range
public optrow as range
public outrow as range
dim ws as worksheet
set ws = activesheet 'set this to the sheet where your selection change event is on

wbcol = ws.UsedRange.Find("Workbook").Column
wscol = ws.UsedRange.Find("Worksheet").Column
nslrow = ws.UsedRange.Find("NSL File").Row
desrow = ws.UsedRange.Find("Design File").Row
tplrow = ws.UsedRange.Find("Template File").Row
parrow = ws.UsedRange.Find("Parameters File").Row
prtrow = ws.UsedRange.Find("Printer Name").Row
optrow = ws.UsedRange.Find("Output paramaters").Row
outrow = ws.UsedRange.Find("Output table file").Row
end sub
This says "Invalid attribute in sub or function" for the "Public" keyword

oops, put the public declarations above the sub declaration
I have tried this but the values are not retained. When the workbook is opened the values are calculated correctly. But when I run the next routine (a selection_change trapper) the variables are empty.
I have just been through a question addressing this problem

https://www.experts-exchange.com/questions/26476634/Excel-2003-How-long-will-a-public-variable-stay-in-memory.html

It states that these variables are reset whenever an End statement is reached. And my program does contain them.

So now my question would be how to terminate my application (macro) abruptly without the End statement? Exit Sub cannot be used because it will return control to the caller routine which is not what is intended.
Saqib,

It may be easier just to have a hidden sheet where you store these settings on Workbook Open

I prefer having the actual settings recorded rather than stored in memory, it's safer and its easy to use

Cheers

Dave
I would not like to add a sheet because this particular has numerous worksheets which are hidden and restored by code and I would like to avoid having an additional sheet in the existing array.

I am now thinking of having a listbox or some other control which can store the values and be hidden.
I would not like to add a sheet because this particular has numerous worksheets which are hidden and restored by code and I would like to avoid having an additional sheet in the existing array.

I am now thinking of having a listbox or some other control which can store the values and be hidden.
ASKER CERTIFIED SOLUTION
Avatar of Dave
Dave
Flag of Australia image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
I don't quite get why the globals on Workbook_open don't work....I've used this method many times over......
>                 Was this comment helpful?   Yes No   MWGainesJR:   I don't quite get why the globals on Workbook_open don't work....I've used this method many times over....

From Saqib's comment the Open worked fine. But other code invokes "End" which then destroys the public variables

Dave

Are the end statements neccessary.  Could something else not be used?   Exit sub or goto's?
As per Sqib's comment
> Exit Sub cannot be used because it will return control to the caller routine which is not what is intended.

Dumping the variables to a location is the best option
I still don't think that is the best option...
Instead of using an end statement why not create a global boolean, and where the end state ment executed set it to true, then when it returns to the caller, have the caller check it
sub caller()
'code

call othersub
if endcalled = true then exit sub
'code

end sub

You can get around using ends....and this allows you to continue using the globals
using globals with a further global workaround is mork work then just dumping to a list

And as above having a list is more robust than storing in memory. In this case the End was killing the Public method, and it doesn't appear that you were aware this would happen. This isn't a shot at you, just noting that variables in memory may be lost

Dave
I have tried the exit sub option. It does work but to an extent. There are instances when the values of the variables have been lost. So I might have to go back to storing the values somewhere in the sheets. At the moment I am busy on something else and have to put this problem pending.

Sorry for the silence

Saqib
Closing