Works but is klunky - help make it elegant

[DChambers]

This code works but I would like to make it more readable and remove excessive or redundant lines. Anyone want to try?

‘Create and prepare the new workbook
   ‘Exit if workbook already exists<?xml:namespace prefix = o ns = "urn:schemas-microsoft-com:office:office" /><o:p></o:p>
      fileExistsStatus = Dir(NewWB)<o:p></o:p>
      If fileExistsStatus <> "" Then<o:p></o:p>
         MsgBox “The Workbook already exists – delete it and restart macro”<o:p></o:p>
         Exit Sub<o:p></o:p>
      End If<o:p></o:p>
   ‘Create workbook with 2 worksheets: Shortage and Reference<o:p></o:p>
      On Error GoTo Err_RestoreDefaultWorksheet<o:p></o:p>
      ‘Save the orininal number of worksheets created with a new workbook<o:p></o:p>
      OrgNumOfWS = Application.SheetsInNewWorkbook<o:p></o:p>
      Application.SheetsInNewWorkbook = 2<o:p></o:p>
      Workbooks.Add<o:p></o:p>
      Worksheets("Sheet1").Name = “Shortage”<o:p></o:p>
      Worksheets("Sheet2").Name = “Reference”<o:p></o:p>
Err_RestoreDefaultWorksheets:
      ‘Restore the original number of worksheets created with a new workbook<o:p></o:p>
      Application.SheetsInNewWorkbook = OrgNumOfWS <o:p></o:p>
<o:p></o:p>

Thanks

 

[xenou]

1)
This isn't too clunky. I prefer readability and clear (self-defining) logic to finding the shortest possible statements - your code is well commented and proceeds in organized and logical steps. That said...

2)
You could separate the DIR() test into a separate function.

2)
Is newWB a string? If so, a naming convention such as strNewWb or sNewWb is a good habit to get into (intCount, lngRow, blnFound, wsMySheet, wbMyWorkbook, dtmMyDate, etc...)

3)
It's curious that you restore the original number of sheets if there's an error, but your code will change the number of sheets permanently if there's no error. Is that what you want? If so, probably no need to error handle - just make the change. Otherwise ...

To change number of sheets only when necessary:

Sub Test()
Dim strNewWBPath As String
Dim wb As Workbook
Dim blnOneSheetInWorkbook As Boolean

'Create and prepare the new workbook
   'Exit if workbook already exists
      If fileExistsStatus(strNewWBPath) Then
         MsgBox "The Workbook already exists – delete it and restart macro"
         Exit Sub
      End If
   
   'Create workbook with 2 worksheets: Shortage and Reference
      If Application.SheetsInNewWorkbook < 2 Then blnOneSheetInWorkbook = True
      Set wb = Workbooks.Add
      wb.Worksheets(1).Name = "Shortage"
      wb.Worksheets(2).Name = "Reference"
      If blnOneSheetInWorkbook Then Application.SheetsInNewWorkbook = 1

End Sub

'----------------------------------------------------------
Function fileExistsStatus(strFullPath As String) As Boolean
    If Dir(strFullPath) > "" Then fileExistsStatus = True
End Function

To restore the number of sheets in all cases:

Dim intOriginalNumberOfSheets As Integer

   'Create workbook with 2 worksheets: Shortage and Reference
      intOriginalNumberOfSheets = Application.SheetsInNewWorkbook
      Application.SheetsInNewWorkbook = 2
      Set wb = Workbooks.Add
      wb.Worksheets(1).Name = "Shortage"
      wb.Worksheets(2).Name = "Reference"
      Application.SheetsInNewWorkbook = intOriginalNumberOfSheets

 

[xenou]

It's curious that you restore the original number of sheets if there's an error

Oops...brushing my teeth tonight it hit me that the line to restore the original number of sheets does execute even without an error. So, no problem there actually.

AB