Attempts to make my code more efficient don't work (1 Viewer)

Zedster

Registered User.
Local time
Today, 02:32
Joined
Jul 2, 2019
Messages
169
I tend to write inefficient code, as an example one of my databases creates an Excel workbook with two sheets one displaying open issues, the other closed. Both sheets are identical in appearance and hence coded identical, the only difference is the query to generate the data. Up until recently I basically repeated the code twice over in the same routine running each in turn with a separate query. But it results in a horrendously big function that is a nightmare to maintain.

So I decided to create a function to produce the content of the worksheet and call it twice with a different query, like below:

Code:
              'Create worksheet 1
870           Call CreateNamedQDF("qdfIssueListByProjectForReportOpen", strSQLIssuesOpen)
872           Set wshOpen = wbk.Sheets(1)
874           wshOpen.Activate
880           Call CreateIssueSheet("qdfIssueListByProjectForReportOpen", wshOpen, "Open and New")
        
              'Create worksheet 2
890           Call CreateNamedQDF("qdfIssueListByProjectForReportClosed", strSQLIssuesClosed)
892           Set wshClosed = wbk.Sheets(2)
894           wshClosed.Activate
900           Call CreateIssueSheet("qdfIssueListByProjectForReportClosed", wshClosed, "Closed")

This worked a treat hence I won't print the hundreds of lines of code.

Within the function CreateIssuesSheet. I noticed again I had repetition, the are 4 main "collections" of columns each with the same format of header and again in my usual inefficiency these were repeated in the sub routine 4 times over. I saw an opportunity to do a similar thins as for the worksheet and create a subroutine which takes a given range and formats it. However this time it fails with error "Object Required" on line 1460.

The call to the subroutine is shown below

Code:
1450      Set rngSelection = wsht.Range("A7:AB7")
1460      FormatSubHeadings (rngSelection)

The routine itself is here

Code:
20        rngSelection.Font.Bold = True
30        With rngSelection.Interior
40            .Pattern = xlSolid
50            .PatternColorIndex = xlAutomatic
60            .ThemeColor = xlThemeColorDark1
70            .TintAndShade = -0.349986266670736
80            .PatternTintAndShade = 0
90        End With
          
100       With rngSelection.Borders(xlEdgeLeft)
110           .LineStyle = xlContinuous
120           .ColorIndex = 0
130           .TintAndShade = 0
140           .Weight = xlMedium
150       End With
160       With rngSelection.Borders(xlEdgeTop)
170           .LineStyle = xlContinuous
180           .ColorIndex = 0
190           .TintAndShade = 0
200           .Weight = xlMedium
210       End With
220       With rngSelection.Borders(xlEdgeBottom)
230           .LineStyle = xlContinuous
240           .ColorIndex = 0
250           .TintAndShade = 0
260           .Weight = xlMedium
270       End With
280       With rngSelection.Borders(xlEdgeRight)
290           .LineStyle = xlContinuous
300           .ColorIndex = 0
310           .TintAndShade = 0
320           .Weight = xlMedium
330       End With

Can anyone advise where I am going wrong?
 

Gasman

Enthusiastic Amateur
Local time
Today, 02:32
Joined
Sep 21, 2011
Messages
14,310
So have you looked to see what rngSelection actually contains?
 

Zedster

Registered User.
Local time
Today, 02:32
Joined
Jul 2, 2019
Messages
169
I hadn't, but I have now, it contains nothing!. Which explains the error, but why would it contain nothing? the wsht object appears valid.
 

Gasman

Enthusiastic Amateur
Local time
Today, 02:32
Joined
Sep 21, 2011
Messages
14,310
That should always be your first point of call.
I seem to recall this issue is on a subsequent run each time?

Qualify the wsht with the excel object name. See what that produces, again by stepping through the code.
 

The_Doc_Man

Immoderate Moderator
Staff member
Local time
Yesterday, 20:32
Joined
Feb 28, 2001
Messages
27,189
This is perhaps just a wild idea, but...

Code:
1460      FormatSubHeadings (rngSelection)


If that is a subroutine call to a sub called FormatSubHeadings then in VBA syntax, those parentheses change the argument variable to a formal expression, which interferes with the argument passage process somehow. Remove the () around the argument. (If I'm wrong, it's a cheap experiment and easily reversed.)
 

Zedster

Registered User.
Local time
Today, 02:32
Joined
Jul 2, 2019
Messages
169
OK thanks to all, you got me thinking. The outcome was interesting. I changed the line referencing the range to qualify it as recommended by Gasman. Ran the code and sure enough rngSelection was no longer nothing which was great. But It was still falling over on the next line. Then I noticed that I had missed out the word "Call" before the subroutine name. So I put that in and all then ran fine. So just as an experiment I returned the rngSelection back to how is was before (when previously it resulted in rngSelection = Nothing), then ran it again and it then ran fine. Makes no sense to me at all! but it is working now and I have a little more experience in debugging thanks to Gasman's suggestion.
 

Users who are viewing this thread

Top Bottom