VBA Best Practice - Format worksheet from another worksheet

Question

I'm looking for a bit of best practice advice on Excel VBA code. I have a report built in Excel, with a control sheet at the front where the user selects a number of variables from drop down boxes and data is compiled based on the selection via SQL through VBA.

The code I have is very simple it copies the data into a worksheet and formats it. However when building the sheet this worked fine as I was stepping through the sheets to makes sure the code was doing what I wanted. Now it is finished I want to perform the code but without it jumping around the worksheets. It looks so much better if it stays on the control sheet until the code has finished.

However I can't seem to perform the same task without referencing the sheet that's being formatted?

Below is the code used on one worksheet to copy data and format. It works up until I need it to select Range("B5:K5").Select then it performs this on my Control sheet.

 On Error Resume Next
    Sheets("Account Details").ShowAllData

Sheets("Account Details").Range("B5:K7500").Cells.ClearContents
Sheets("Account Details").Range("B5:K7500").Borders.LineStyle = xlNone

Sheets("Account Details").Range("B5").CopyFromRecordset rst2

Sheets("Account Details").Range("B5:K5").Select
Sheets("Account Details").Range(Selection, Selection.End(xlDown)).Select

Sheets("Account Details").Selection.Borders(xlDiagonalDown).LineStyle = xlNone
Sheets("Account Details").Selection.Borders(xlDiagonalUp).LineStyle = xlNone
Sheets("Account Details").Selection.Borders(xlEdgeLeft).LineStyle = xlContinuous
Sheets("Account Details").Selection.Borders(xlEdgeRight).LineStyle = xlContinuous
Sheets("Account Details").Selection.Borders(xlEdgeTop).LineStyle = xlContinuous
Sheets("Account Details").Selection.Borders(xlEdgeBottom).LineStyle = xlContinuous
Sheets("Account Details").Selection.Borders(xlInsideVertical).LineStyle = xlContinuous
Sheets("Account Details").Selection.Borders(xlInsideHorizontal).LineStyle = xlContinuous

Sheets("Account Details").Range("A1").Select

The second part I wanted to ask was on VBA best practice. Not that I know how, but I can't help thinking that keep referencing the worksheet is a little messy and not really needed. I've tried a couple of With statements but with no joy.

Any help would be much appreciated.

Thanks in advance!

Well, first comment is avoid select, see https://stackoverflow.com/q/10714251/4961700 — Solar Mike, Feb 01 '19 at 12:45
VBA best practise 101: **Do not use** `On Error Resume Next` — FunThomas, Feb 01 '19 at 12:55
Name your Sheets. Like `Dim ws as thisworkbook.sheets("Account Details"). So that you only need to type `ws.` instead of `Sheets("Account Details")` — Luuklag, Feb 01 '19 at 12:59
Thanks guys, great comments/advice and thanks for the article link. — Carlos80, Feb 01 '19 at 13:17

Harassed Dad · Accepted Answer · 2019-02-01T14:58:19.487

1

Using your code as an example

   '  On Error Resume Next  'NO - never use this, it doesn't deal with errors, just hides them - it will bite you eventually
 With Sheets("Account Details")
      IF .Autofiltermode Then .ShowAllData

      with .Range("B5:K7500")  'note dot at beginning
          .Cells.ClearContents
          .Borders.LineStyle = xlNone
      End With
      Dim r as range
      set r = .range("b5")
      r.CopyFromRecordset rst2  'this is the first cell in b5:k7500


     Set r = .Range(r, r.End(xlDown).end(xltoright))  'redefine r to point to area
     with r
         .Borders(xlDiagonalDown).LineStyle = xlNone
         .Borders(xlDiagonalUp).LineStyle = xlNone
         .Borders(xlEdgeLeft).LineStyle = xlContinuous
         .Borders(xlEdgeRight).LineStyle = xlContinuous
         .Borders(xlEdgeTop).LineStyle = xlContinuous
         .Borders(xlEdgeBottom).LineStyle = xlContinuous
         .Borders(xlInsideVertical).LineStyle = xlContinuous
         .Borders(xlInsideHorizontal).LineStyle = xlContinuous
    End With  'end of with r
 End With  'end if with sheet

'Sheets("Account Details").Range("A1").Select 'unnecessary

edited Feb 01 '19 at 14:58

answered Feb 01 '19 at 12:59

Harassed Dad

4,669
1
10
12

Thanks Harassed Dad, this is perfect. I appreciate my code is very basic it was a combination of Google searches and recorded macros until I got it doing what I wanted. However, the only error I am getting on your code is the .ShowAllData part which gives me a Run-time error 1004. Application-defined or object-defined error. What I am trying to do here is if there is a filter on the table clear it if not leave it. It's designed to be a bit of error handling. – Carlos80 Feb 01 '19 at 13:17
I suspect that was always erroring, your on error line was hiding it. Try using If .AutoFilterMode then .ShowAllData – Harassed Dad Feb 01 '19 at 13:24
Thanks Harassed Dad, I tried .AutoFilterMode but got the same error. I think I might just remove the filter and let the user scroll. – Carlos80 Feb 01 '19 at 14:11
I've corrected the code anyway, in case you misunderstood my comment - I think this should prevent the error as written – Harassed Dad Feb 01 '19 at 14:59
Hi Harassed Dad, I have a similar issue with an Activecell loop being called from the control sheet. Can I message you on help with this? I'm guessing bad practice to add to this thread. Thanks – Carlos80 Feb 01 '19 at 15:00
Best practice is to start a new question - that way someone else may be able to answer before I'm back - I only hit SO while waiting for stuff to finish running so it may be days before i look again – Harassed Dad Feb 01 '19 at 15:05

VBA Best Practice - Format worksheet from another worksheet

1 Answers1