Simplifying Code

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

Code:

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.

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.

Code:

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

Code:

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

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,

Code:

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.

Maybe something like this?

Code:

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

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.

Assigning a Range to a variable includes the sheet if present

Code:

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

Worksheet Code

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

Code:

Sub Update(Rng As Range) ' ' ' Sub Archive(Rng As Range) ' ' '

To incorporate what Paul suggested as to consolidating all common factors

Code:

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

Code:

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

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.

Quote:

Originally Posted by Slicemahn

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