Is there a better way to write this?

glasgowlad1999

Registered User.
Local time
Today, 08:02
Joined
Jun 17, 2013
Messages
27
Is there a better way to write this, this is what I get when I convert from macro to vb in access 2010. It runs but doesnt export the files.


Code:
Private Sub LindasWS_Click()
On Error GoTo LindasWS_Err

    DoCmd.SetWarnings False
     DoCmd.OpenQuery "tblProcedureAppend", acViewNormal, acEdit
    DoCmd.OpenQuery "tblDiagnosisAppend", acViewNormal, acEdit
    DoCmd.OpenQuery "LDISTINCTCDC", acViewNormal, acEdit
    DoCmd.OpenQuery "LDISTINCTDC", acViewNormal, acEdit
    
    RunLWSCDC
    RunLWSDC
    RunLWSCA
    RunLWSCPC
    RunLWSFSA
    RunLWSPA
    RunLWSPC
    
     If (Forms!AIR_DetailsB!UserName = "rf") Then
        DoCmd.RunMacro "openexcelRandy", , ""
    If (Forms!AIR_DetailsB!UserName = "jb") Then
        DoCmd.RunMacro "openexcelJosephine", , ""
    If (Forms!AIR_DetailsB!UserName = "tk") Then
        DoCmd.RunMacro "openexcelTammy", , ""
    If (Forms!AIR_DetailsB!UserName = "li") Then
        DoCmd.RunMacro "openexcelLindaP", , ""
    If (Forms!AIR_DetailsB!UserName = "jo") Then
        DoCmd.RunMacro "openexcelDonna", , ""
    If (Forms!AIR_DetailsB!UserName = "tm") Then
        DoCmd.RunMacro "SaveAuditWorksheet", , ""
    Else
        Beep
        MsgBox "Your Security role doesn't allow to export the Auditors Worksheet!", vbOKOnly, ""
        End
    End If
    End If
    End If
    End If
    End If
    End If
    
    DoCmd.OpenQuery "deleteLWorkSheeCDC", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetCPCRaw", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetCA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetCPC", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetDC", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetDCRaw", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetFSA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetPA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetPC", acViewNormal, acEdit
    DoCmd.OpenQuery "deletetblDiagnosis", acViewNormal, acEdit
    DoCmd.OpenQuery "deletetblProcedure", acViewNormal, acEdit
    DoCmd.SetWarnings True

 Beep
        MsgBox "Your Auditors Worksheet has been created in your folder", vbOKOnly, ""
        End
        
LindasWS_Exit:
    Exit Sub

LindasWS_Err:
    MsgBox Error$
    Resume LindasWS_Exit
End Sub
 
Last edited:
I don't see anything there that would export a file, unless it's in the macros or whatever the "Run..." items are, presumably functions. I would point out a logical flaw, which may be more visible with indenting:

Code:
  If (Forms!AIR_DetailsB!UserName = "rf") Then
    DoCmd.RunMacro "openexcelRandy", , ""
    If (Forms!AIR_DetailsB!UserName = "jb") Then
      DoCmd.RunMacro "openexcelJosephine", , ""
      If (Forms!AIR_DetailsB!UserName = "tk") Then
        DoCmd.RunMacro "openexcelTammy", , ""
        If (Forms!AIR_DetailsB!UserName = "li") Then
          DoCmd.RunMacro "openexcelLindaP", , ""
          If (Forms!AIR_DetailsB!UserName = "jo") Then
            DoCmd.RunMacro "openexcelDonna", , ""
            If (Forms!AIR_DetailsB!UserName = "tm") Then
              DoCmd.RunMacro "SaveAuditWorksheet", , ""
            Else
              Beep
              MsgBox "Your Security role doesn't allow to export the Auditors Worksheet!", vbOKOnly, ""
              End
            End If
          End If
        End If
      End If
    End If
  End If
 
This code gets the data ready in temp tables to export

Code:
  DoCmd.OpenQuery "tblProcedureAppend", acViewNormal, acEdit
    DoCmd.OpenQuery "tblDiagnosisAppend", acViewNormal, acEdit
    DoCmd.OpenQuery "LDISTINCTCDC", acViewNormal, acEdit
    DoCmd.OpenQuery "LDISTINCTDC", acViewNormal, acEdit
    
    RunLWSCDC
    RunLWSDC
    RunLWSCA
    RunLWSCPC
    RunLWSFSA
    RunLWSPA
    RunLWSPC

This code runs the correct Marco to export

Code:
If (Forms!AIR_DetailsB!UserName = "rf") Then
    DoCmd.RunMacro "openexcelRandy", , ""
    If (Forms!AIR_DetailsB!UserName = "jb") Then
      DoCmd.RunMacro "openexcelJosephine", , ""
      If (Forms!AIR_DetailsB!UserName = "tk") Then
        DoCmd.RunMacro "openexcelTammy", , ""
        If (Forms!AIR_DetailsB!UserName = "li") Then
          DoCmd.RunMacro "openexcelLindaP", , ""
          If (Forms!AIR_DetailsB!UserName = "jo") Then
            DoCmd.RunMacro "openexcelDonna", , ""
            If (Forms!AIR_DetailsB!UserName = "tm") Then
              DoCmd.RunMacro "SaveAuditWorksheet", , ""
            Else
              Beep
              MsgBox "Your Security role doesn't allow to export the Auditors Worksheet!", vbOKOnly, ""
              End
            End If
          End If
        End If
      End If
    End If
  End If

This code deletes the temp tables
Code:
DoCmd.OpenQuery "deleteLWorkSheeCDC", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetCPCRaw", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetCA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetCPC", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetDC", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetDCRaw", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetFSA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetPA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetPC", acViewNormal, acEdit
    DoCmd.OpenQuery "deletetblDiagnosis", acViewNormal, acEdit
    DoCmd.OpenQuery "deletetblProcedure", acViewNormal, acEdit
    DoCmd.SetWarnings True

 Beep
        MsgBox "Your Auditors Worksheet has been created in your folder", vbOKOnly, ""
        End
 
Are you seeing the logical flaw? If the user name isn't "rf", none of the other tests will run. I'm guessing you get an export if the user is rf. You want to use either Select/Case or ElseIf.
 
I have 10 users and each have their own location to export the file too. So each if will run the Marco that holds their location.
 
I must not be saying this well. The way the code is written, the only way the others could run is if "rf" is the user, which is logically impossible. Please look at Select/Case in VBA help and use that.

I should add that in a production db you wouldn't have all that in code, as it would mean you'd have to change the db design every time you got a new user. The queries and such should be parameter based, so only one runs, but it uses the user.
 
I dont see a flaw and I am getting the last message, it just doesn't export.
 
Have you tried for user rf? Everything in red is contained within the test for rf, thus NONE of it will run if the user is not rf.

Code:
  If (Forms!AIR_DetailsB!UserName = "rf") Then
[COLOR="Red"]    DoCmd.RunMacro "openexcelRandy", , ""
    If (Forms!AIR_DetailsB!UserName = "jb") Then
      DoCmd.RunMacro "openexcelJosephine", , ""
      If (Forms!AIR_DetailsB!UserName = "tk") Then
        DoCmd.RunMacro "openexcelTammy", , ""
        If (Forms!AIR_DetailsB!UserName = "li") Then
          DoCmd.RunMacro "openexcelLindaP", , ""
          If (Forms!AIR_DetailsB!UserName = "jo") Then
            DoCmd.RunMacro "openexcelDonna", , ""
            If (Forms!AIR_DetailsB!UserName = "tm") Then
              DoCmd.RunMacro "SaveAuditWorksheet", , ""
            Else
              Beep
              MsgBox "Your Security role doesn't allow to export the Auditors Worksheet!", vbOKOnly, ""
              End
            End If
          End If
        End If
      End If
    End If[/COLOR]
  End If
 
I will add that you could save yourself a ton of headaches by simplifying at least a part of that ugly if-ladder.

If you have a split Front-End/Back-End database, the CurrentDB.Name property is the fully qualified specification of that database. You can parse out the device and path from this string and build a path for the spreadsheet.

Now, if this is an FE/BE situation that resides on a domain that uses formal logins of any kind, you can do yourself one more favor. Each person who logs in does so in a way that, for domain-resident users, provides their login names. So make a table in the BE part that includes the login name and a set of T/F flags as to what that user can do, or make it role based if no user is ever multi-role. Then use the Environ("Username") function to see who it was that logged in and do a lookup on that name in your user table.

Now you have two things to do to decide if/where to build your workbooks. Look up the username to drive a DLookup to determine IF you will build the workbook. Parse the CurrentDB.Name to decide WHERE to build the workbook. All of those "IF" ladders will eat your socks if your auditors list ever exceeds 16 users.

Note: This ONLY works if you hide the Navigation pane from the user and drive this whole thing via forms that act as a switch-board or dispatcher form. If your users can see the database entities via the navigation pane, you were already hosed anyway because in that case you wouldn't be able to stop someone from building your Auditor's workbook anyway.
 
Apart from the logic errors here, I would put a block like this in a table . . .
Code:
     If (Forms!AIR_DetailsB!UserName = "rf") Then
        DoCmd.RunMacro "openexcelRandy", , ""
    If (Forms!AIR_DetailsB!UserName = "jb") Then
        DoCmd.RunMacro "openexcelJosephine", , ""
    If (Forms!AIR_DetailsB!UserName = "tk") Then
        DoCmd.RunMacro "openexcelTammy", , ""
    If (Forms!AIR_DetailsB!UserName = "li") Then
        DoCmd.RunMacro "openexcelLindaP", , ""
    If (Forms!AIR_DetailsB!UserName = "jo") Then
        DoCmd.RunMacro "openexcelDonna", , ""
    If (Forms!AIR_DetailsB!UserName = "tm") Then
        DoCmd.RunMacro "SaveAuditWorksheet", , ""
    Else
so that if I have to add a user, I can do so in the table. Fields might be . . .
tUser
UserID (PK)
Name
Initials
openexcelParameter
Then, that if block is condensed to . . .
Code:
dim var
var = dlookup("openexcelParameter", "tUser", "Initials = '" & forms!AirDetailB.Username & "'")
if isnull(var) then 
    beep
    msgbox "failed"
else
    docmd.runmacro var, , ""
else

I would also put this list of names in a table, open a recordset, and do this in a loop . . .
Code:
    DoCmd.OpenQuery "deleteLWorkSheeCDC", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetCPCRaw", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetCA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetCPC", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetDC", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetDCRaw", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetFSA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetPA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetPC", acViewNormal, acEdit
    DoCmd.OpenQuery "deletetblDiagnosis", acViewNormal, acEdit
    DoCmd.OpenQuery "deletetblProcedure", acViewNormal, acEdit
 
markk is openexcelParameter where i store DoCmd.RunMacro "openexcelDonna", , ""
 
The DoCmd.RunMacro command is code, in red below. The parameter value, say "openexcelTammy", is data, stored in a table in Tammy's user record.
Code:
dim var
var = dlookup("openexcelParameter", "tUser", "Initials = '" & forms!AirDetailB.Username & "'")
if isnull(var) then 
    beep
    msgbox "failed"
else
    [B][COLOR="DarkRed"]docmd.runmacro var, , ""[/COLOR][/B]
else
But my guess is, looking at your other code, that you probably have a few differently named macros that do almost exactly the same thing. What does the macro "openexcelDonna" do? And how does it differ from "openexcelTammy?" Probably that could be simplified too, along the same lines as these other structures.
 
Only difference between Tammy and Donna is the url location of their folder in the directory.
 
This code takes the data and puts in a temp table
Code:
 DoCmd.OpenQuery "tblProcedureAppend", acViewNormal, acEdit
    DoCmd.OpenQuery "tblDiagnosisAppend", acViewNormal, acEdit
    DoCmd.OpenQuery "LDISTINCTCDC", acViewNormal, acEdit
    DoCmd.OpenQuery "LDISTINCTDC", acViewNormal, acEdit
This code is a loop to combine into single cells, one runCODE loop for each of the 8 different cell
Code:
    RunLWSCDC
    RunLWSDC
    RunLWSCA
    RunLWSCPC
    RunLWSFSA
    RunLWSPA
    RunLWSPC

They have worked fine for a single user, now I need to auto save to a folder depending on the user

This code is just deleting all from all the temp tables:
Code:
DoCmd.OpenQuery "deleteLWorkSheeCDC", acViewNormal, acEdit

'' deleteLWorkSheeCDC is only sql: 
DELETE LWorkSheeCDC.*
FROM LWorkSheeCDC;


    DoCmd.OpenQuery "deleteLWorkSheetCPCRaw", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetCA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetCPC", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetDC", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetDCRaw", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetFSA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetPA", acViewNormal, acEdit
    DoCmd.OpenQuery "deleteLWorkSheetPC", acViewNormal, acEdit
    DoCmd.OpenQuery "deletetblDiagnosis", acViewNormal, acEdit
    DoCmd.OpenQuery "deletetblProcedure", acViewNormal, acEdit
 
Using this code I get a run-time error 2485 MS Access cannon find the object 'AuditWorkSheet

Inside openexcelParameter i have the macro name AuditWorkSheet
Code:
Private Sub LindasWS_Click()
Dim var
var = DLookup("openexcelParameter", "dbo_tblUser", "UserName = '" & Forms!AIR_DetailsB.UserName & "'")
If IsNull(var) Then
    Beep
    MsgBox "failed"
Else
    DoCmd.RunMacro var, , ""

End If
    
    
End Sub
 
Last edited:
What line causes the error? What is the value of var after the DLookup()?
Only difference between Tammy and Donna is the url location of their folder in the directory.
Why not put the URL of the users folder in the users table? Then you don't need many named macros, you can just pass the folder URL to a generic routine?

See, the general principle for you to get is that you should never hard code data. You should use variables, and write routines that consume the variables. Then all your code get's way simpler, and you can change what your code does simply by changing the data you give it, rather than re-writing it.
 
The line that causes the error is:

Code:
DoCmd.RunMacro var, , ""

var = "AuditWorkSheet'

I typed how it was displayed

Markk I will try the other way once I get my head around this way. However I am very new to VBA and I am now changing my macros to VBA.
 
Last edited:
Well, it would appear that there is no macro with that name. Are you sure it's a macro and not a VBA sub or function?
 
Under Macros I have AuditWorkSheet

In the table I have AuditWorkSheet not "AuditWorkSheet" or "AuditWorkSheet' or 'AuditWorkSheet'
 
is there a way to add a trim to the code as it looks like the SQL db is adding spaces
Code:
DoCmd.RunMacro var, , ""
 
Last edited:

Users who are viewing this thread

Back
Top Bottom