Simplifying Code

[Slicemahn]

Hi VBAExpress Nation!

I am hoping to get some help with simplifying some code. It is long and I have spared everyone's eyes from the long reading. Essentially what I would like to do is have the main loop execute from the worksheet but the cell checks from modules. I am not quite sure how I would reference the cells to the modules and have the checks executed. See the code below

If SPFile_Exists(Cells(x, 24).Value) = False Then              Application.StatusBar = "Report " & Cells(x, 2).Value & " cannot be archived"
              Cells(x, 12).Value = "Error  - File Not Found"
              Cells(x, 14).Value = Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")
              Cells(x, 15).Value = Format(Cells(x, 14) - Cells(x, 13), "hh:mm:ss")
              ErrorLog.WriteLog Report:=Cells(x, 2).Value, _
              ErrorMsg:=Cells(x, 11).Value & " TASK NOT COMPLETED" & "|" & Cells(x, 12).Value, _
              Location:=Cells(x, 24).Value, _
              Description:="Check the URL file location cannot be found"
              ReportErrors = ReportErrors + 1
        Else
        If ReportCheckOut(Cells(x, 24)) = False Then
              Application.StatusBar = "Report " & Cells(x, 2).Value & " cannot be checked out."
              Cells(x, 12).Value = "Error - Checked Out Report"
              Cells(x, 14).Value = Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")
              Cells(x, 15).Value = Format(Cells(x, 14) - Cells(x, 13), "hh:mm:ss")
              ErrorLog.WriteLog Report:=Cells(x, 2).Value, _
              ErrorMsg:=Cells(x, 11).Value & " TASK NOT COMPLETED" & "|" & Cells(x, 12).Value, _
              Location:=Cells(x, 24).Value, _
              Description:="Report is currently checked out to another user."
              ReportErrors = ReportErrors + 1
        Else
        If Cells(x, 5) <> "Archived" Then
              Application.StatusBar = "Report " & Cells(x, 2).Value & " cannot be reactivated"
              Cells(x, 12).Value = "Error - Active Report"
              Cells(x, 14).Value = Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")
              Cells(x, 15).Value = Format(Cells(x, 14) - Cells(x, 13), "hh:mm:ss")
              ErrorLog.WriteLog Report:=Cells(x, 2).Value, _
              ErrorMsg:=Cells(x, 11).Value & " TASK NOT COMPLETED" & "|" & Cells(x, 12).Value, _
              Location:=Cells(x, 24).Value, _
              Description:="Report is already active"
              ReportErrors = ReportErrors + 1
        Else
              Cells(x, 5).Value = "Active"
              Cells(x, 12).Value = "Success"
              UpdateMeta Cells(x, 24).Value, Cells(x, 11).Value
              Cells(x, 13).Value = Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")
              Cells(x, 14).Value = Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")
              Cells(x, 15).Value = Format(Cells(x, 14) - Cells(x, 13), "hh:mm:ss")
              Application.StatusBar = "Report " & Cells(x, 2).Value & " is now reactivated"
              ReportSuccess = ReportSuccess + 1
        End If
        End If
        End If

So this sample code is in the event that a user selects "Update" for the action taken. There are also "Archive" and "Refresh" actions that are also choices a user can make. In the module do I reference ActiveCell? Do I do it in a With block? I am not sure. This is the reason why I am reaching out to the knowledgeable VBAExpress Nation for help. Thanks.

[Paul_Hossler]

My 2 cents

1. Not sure about your logic flow, but when I make the indents more prominent it looks like there are 3 levels if If Then / Else

2. I never just use Cells without an explicit parent reference, with via a With or Worksheets("Sheet1").Cells(X,14)
Without that parent reference it will default to whatever the Activesheet is

3.

Cells(x, 14).Value = Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")

is common in all pieces, so you could move that to the top and use one time

If SPFile_Exists(Cells(x, 24).Value) = False Then
    Application.StatusBar = "Report " & Cells(x, 2).Value & " cannot be archived"
    Cells(x, 12).Value = "Error  - File Not Found"
    Cells(x, 14).Value = Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")
    Cells(x, 15).Value = Format(Cells(x, 14) - Cells(x, 13), "hh:mm:ss")
    ErrorLog.WriteLog Report:=Cells(x, 2).Value, _
        ErrorMsg:=Cells(x, 11).Value & " TASK NOT COMPLETED" & "|" & Cells(x, 12).Value, _
       Location:=Cells(x, 24).Value, Description:="Check the URL file location cannot be found"
    ReportErrors = ReportErrors + 1


Else
    If ReportCheckOut(Cells(x, 24)) = False Then
          Application.StatusBar = "Report " & Cells(x, 2).Value & " cannot be checked out."
          Cells(x, 12).Value = "Error - Checked Out Report"
          Cells(x, 14).Value = Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")
          Cells(x, 15).Value = Format(Cells(x, 14) - Cells(x, 13), "hh:mm:ss")
          ErrorLog.WriteLog Report:=Cells(x, 2).Value, _
              ErrorMsg:=Cells(x, 11).Value & " TASK NOT COMPLETED" & "|" & Cells(x, 12).Value, _
              Location:=Cells(x, 24).Value, Description:="Report is currently checked out to another user."
          ReportErrors = ReportErrors + 1
    
    Else
        If Cells(x, 5) <> "Archived" Then
              Application.StatusBar = "Report " & Cells(x, 2).Value & " cannot be reactivated"
              Cells(x, 12).Value = "Error - Active Report"
              Cells(x, 14).Value = Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")
              Cells(x, 15).Value = Format(Cells(x, 14) - Cells(x, 13), "hh:mm:ss")
              ErrorLog.WriteLog Report:=Cells(x, 2).Value, _
                  ErrorMsg:=Cells(x, 11).Value & " TASK NOT COMPLETED" & "|" & Cells(x, 12).Value, _
                  Location:=Cells(x, 24).Value, Description:="Report is already active"
              ReportErrors = ReportErrors + 1
        
        Else
              Cells(x, 5).Value = "Active"
              Cells(x, 12).Value = "Success"
              UpdateMeta Cells(x, 24).Value, Cells(x, 11).Value
              Cells(x, 13).Value = Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")
              Cells(x, 14).Value = Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")
              Cells(x, 15).Value = Format(Cells(x, 14) - Cells(x, 13), "hh:mm:ss")
              Application.StatusBar = "Report " & Cells(x, 2).Value & " is now reactivated"
              ReportSuccess = ReportSuccess + 1
        End If
    End If
End If

[Slicemahn]

Thanks Paul. This is only a snip of the code. I appreciate the pointers and have used this method in my code. But, what I am trying to do is to place this code in a module and operate the main code from the worksheet. So, the code above is for Updating a file. There are two other actions in which a user can select: Refresh and Archive, which has code just as lengthy. Couldn't I place these lengthy codes in modules and then with a code for the worksheet object call those sub procedures to execute as above?

That is,

With ThisWorkbook.Worksheets("Reports")
       ReportCount = .[B2].CurrentRegion.Rows.Count
       SelectedReports = Application.WorksheetFunction.CountA(Columns(11)) - 1
End With

X=3:Y=0:ReportErrors=0:ReportSuccess=0

Do While x<=Report Count

Application.ScreenUpdating = False

Select Case Sheet1.Cells(x,11).Value
Case Update

Call Update Procedure

Case Archive

    Call Archive Procedure

Case Refresh

    Call Refresh Procedure

End Select

Application.ScreenUpdating = True

x = x + 1

Loop

This would be the structure, but what I need help in doing is having the sub procedures execute according to the cells in the loop.

[Paul_Hossler]

Maybe something like this?

With ThisWorkbook.Worksheets("Reports")       ReportCount = .[B2].CurrentRegion.Rows.Count
       SelectedReports = Application.WorksheetFunction.CountA(Columns(11)) - 1
End With


x = 3: Y = 0: ReportErrors = 0: ReportSuccess = 0


    Do While x <= ReportCount


        Application.ScreenUpdating = False


        Select Case Worksheets("Reports").Cells(x, 11).Value
            
            Case "Update"
                Call UpdateProcedure
    
            Case "Archive"
                Call ArchiveProcedure
        
            Case "Refresh"
                Call RefreshProcedure


        End Select


    Application.ScreenUpdating = True


    x = x + 1


Loop

[Slicemahn]

How do I write the code that will be placed in a module to execute from the sub procedure from the workbook object? All you are showing me is code structure, which I know already. I may have not posted in the format that you expected but format is not question being asked here.

So again, the first code I posted is a snip of an elaborate code that is very long. I didn't post the entire code because I am sure I would not get any responses. What I want to do is place the elaborate code in Modules. Then write code like the above to call the procedures. That seems simple; however, the challenge for me is writing the code in the modules so that it executes. I am looping through a whole list of items. I just want the code for the worksheet to handle the loop and call the various procedures as the case scenarios present themselves. My challenge is writing the code for the modules because I am difficulty (a) referencing the cell to be updated; and,(b) other cells to be updated. So please, if someone can help me write one of the procedures (Update, Archive or Refresh) using the code from my original post, I would greatly appreciate it. Thanks.

[SamT]

Assigning a Range to a variable includes the sheet if present

Set Rng = Sheets("Sheet1").Range("A1")

Worksheet Code

Set Rng = Sheet1.Cells(x,11)

Select Case Rng.Value
   Case = "Update":   UpDate Rng
   Case = "Archive":  Archive Rng
   Case = "Refresh":  Refresh Rng
End Select

Sample Module Code Heads

Sub Update(Rng As Range)
'
'
'

Sub Archive(Rng As Range)
'
'
'

To incorporate what Paul suggested as to consolidating all common factors

Case = "Update"
   Common Rng
   Update Rng
Case ' ' '
'
'
'

Sub Common(Rng As Range)
   With Rng
      .Offset(, 2) =Format(Now, "mm/dd/yyyy hh:mm:ss AM/PM")
      .Offset(, 3)= Format(.Offset(, 2) - .Offset(, 1), "[hh]:mm:ss")
   End With
End Sub

[Slicemahn]

Hi Sam T,

Thank you. Finally someone who knows what I am asking. (Raising hands to the sky). I will try this out and post if I have any further issues.

[Aussiebear]

All you are showing me is code structure, which I know already. I may have not posted in the format that you expected but format is not question being asked here.

Paul has clearly suggested how to go about posting your code, SamT has more clearly defined "how" to structure your code. Both are on the same page but to different degrees.



For example
Code Main
'Do this or that
'decision time
Call Process 1
Does Process 1 need further Options
Call Option1 or
Cal Option 2
Exit to Code Main
End Code Main