Load a drawing in Microsoft Visio and show the proper window to the user

Question

In this scenario, I have VBA code and use Late binding and no error handling for simplification. Also, my question is not explicitly related to Visio, it could be any else application. Let's just accept that the steps are required and my question really belongs naming and architecture.

Goal of the code

Load a drawing in Microsoft Visio using VBA and show the proper window to the user.

Steps

• Attach to running Visio instance (GetObject). Or, if none exists, start a new Visio instance (CreateObject).

• Bring Visio instance to front. This is necessary, to avoid Visio blinking in the taskbar after loading a visio file

• Load Visio file

• Prevent Visio screen updating to avoid screen flickering caused by my following code

• Sync some data. Whatever will be done with the drawing, doesn't really matter here

• Determine desired Visio window. Loading a Visio file could cause multiple windows of that drawing being opened at once, but there is one specific I want to show

• Activate desired Visio window

• Maximize desired Visio window. An attached Visio instance could be in non-maximized window mode, but I want maximized

• Resume Visio screen updating

Questions

1. How would you name the 'main' procedure, in my example OpenVisioDrawing? As you can see, there also is a helper procedure named LoadVisioDrawing, which conflicts a bit with OpenVisioDrawing. But naming OpenVisioDrawing more complicated like AssureVisioInstanceLoadDrawingAndShowItToTheUser isn't really nice.

   This naming problem seems to happen more often to me, when finding short pregnant names for 'assembly procedures' (procedures which do many steps).

2. Or would you solve this naming problem by refactoring my approach?

3. Would you suggest a different architecture at all, considering application instantiating, screen updating, loading, syncing, activating, viewing?

Module TestVisioInterface

Option Compare Database
Option Explicit

Sub Test()
    With New VisioInterface
        .OpenVisioDrawing "C:\myVisioFile.vsd", "Custom drawing title"
    End With
End Sub

Class VisioInterface

Option Compare Database
Option Explicit

Private Const VISIO_CLASS_ID As String = "Visio.Application"

Public Sub OpenVisioDrawing(ByVal filename As String, someData As String)
    Dim app As Object
    Set app = GetVisioInstance()

    ActivateVisioInstance app

    Dim drawing As Object
    Set drawing = LoadVisioDrawing(app, filename)

    VisioScreenUpdating app, False

    SyncVisioDrawing drawing, someData

    Dim window As Object
    Set window = DetermineDesiredVisioWindow(app, drawing)

    ActivateVisioWindow window

    MaximizeVisioWindow window

    VisioScreenUpdating app, True
End Sub

Private Function GetVisioInstance() As Object
    If VisioInstanceExist() Then
        Set GetVisioInstance = AttachVisioInstance()
    Else
        Set GetVisioInstance = StartVisioInstance()
    End If
End Function

Private Function VisioInstanceExist() As Boolean
    On Error Resume Next
    Dim visioApp As Object
    Set visioApp = GetObject(, VISIO_CLASS_ID)
    VisioInstanceExist = Not visioApp Is Nothing
End Function

Private Function AttachVisioInstance() As Object
    Set AttachVisioInstance = GetObject(, VISIO_CLASS_ID)
End Function

Private Function StartVisioInstance() As Object
    Set StartVisioInstance = CreateObject(VISIO_CLASS_ID)
End Function

Private Sub ActivateVisioInstance(ByRef app As Object)
    AppActivate app.window.Caption
End Sub

Private Function LoadVisioDrawing(ByRef app As Object, ByVal filename As String) As Object
    Const visOpenRW = &H20
    Set LoadVisioDrawing = app.Documents.OpenEx(filename, visOpenRW)
End Function

Private Sub SyncVisioDrawing(ByRef drawing As Object, ByVal someData As String)
    drawing.Title = someData
End Sub

Private Function DetermineDesiredVisioWindow(ByRef app As Object, ByRef drawing As Object) As Object
    Dim window As Object
    For Each window In app.Windows
        If window.Document Is drawing Then
            Set DetermineDesiredVisioWindow = window
            If window.Caption Like "*:2 *" Then
                Set DetermineDesiredVisioWindow = window
                Exit Function
            End If
        End If
    Next
End Function

Private Sub ActivateVisioWindow(ByRef window As Object)
    window.Activate
End Sub

Private Sub MaximizeVisioWindow(ByRef window As Object)
    Const visWSMaximized = &H40000000
    window.WindowState = visWSMaximized
End Sub

Private Sub VisioScreenUpdating(ByRef app As Object, ByVal allow As Boolean)
    app.ScreenUpdating = allow
End Sub

Answer 1

Naming

Although the VisioInterface does interface with Visio, it is not an Interface. It you add more functionality to the class I would name it VisioDocument

I agree with @Raystafarian when it comes to the naming of your procedures. Everything in this class is related to Visio. Adding Visio to the names of all the procedures is just clutter.

I agree with @Raystafarian when it comes to the naming of visWSMaximized and visOpenRW. These values may have constant values but they are actually Enumerations. Consider adding the Enumerations to the head of the class. Make them Public if you are going to use them in a parameter of Public Procedure, otherwise make them Private.

' https://msdn.microsoft.com/en-us/library/aa342166(v=office.12).aspx
Public Enum VisWindowStates
    visWSActive = &H4000000
    visWSMaximized = &H40000000
    visWSMinimized = &H20000000
End Enum

Program Flow

The class is somewhat clunky. It needs some Public Fields to bring it together. Document and maybe even Windows field will make all the procedure gel better. For example, you have both Load and Open procedure but Activate uses AppActivate to activate the Window instead of directly referencing it.

There is no clear starting point to using the class. Consider writing some sort Initiate or Init procedure. I would replace LoadVisioDrawing with Init(Path as String).

You should consider using GetObject(PATH) to set a direct reference to the file if it already open. For example: Set Document = GetObject("C:\myVisioFile.vsd") will either set a reference the myVisioFile file or throw an error.

Don't Reinvent the Wheel - Model Your Code After Existing Patterns

I try to model my classes after the Applications, Objects, and procedures that they are Interfacing with or mimicking. Consider ScreenUpdating, in both Excel and Visio ScreenUpdating is a property that takes an Enumeration. When I were to write a ScreenUpdating procedure in a class module, guess what, I write it as a property that takes an Enumeration.

Does Visio load a Document or does it Open a Document? It opens one of course. Anyone familiar with any of the Microsoft Application Models knows this. If they see OpenDocument they instantly know what the procedure is expected to do. On the other hand when I seen LoadVisioDrawing, I had to discern the meaning of both "Load" and "Drawing". Because I have never used Visio and don't have access to it, I had to research VSD to determine that it was a Visio Document format. It might be advantageous use the more generic name Document.

Modeling your code after existing patterns will make your code more understandable to others and easier to re-assimilate if you have to modify it down the road. Familiarity makes understanding and learning far easier tasks.

VisioDocument - VisioInterface Refactored

What follows is rough draft of how I would write the class. I could not test it because I don't have Visio installed but I am confident that it would not be hard to get it up and running.

Option Explicit
Public Enum VisWindowStates
    visWSActive = &H4000000
    visWSMaximized = &H40000000
    visWSMinimized = &H20000000
End Enum

Public mVisioApp As Object
Public mDocument As Object
Public mWindow As Object
Private mScreenUpdating As Boolean
Private mWindowState As XlWindowState
Private mPath As String

Public Function Init(Optional PATH As String) As Boolean
    Const VISIO_CLASS_ID As String = "Visio.Application"
    Dim doc As Object
    mPath = PATH
    If Len(mPath) <> 0 Then
        If Len(Dir(mPath)) = 0 Then
            MsgBox "File Not Found"
            Exit Function
        End If
    End If

    On Error Resume Next
    Set mDocument = GetObject(mPath)
    On Error GoTo 0

    If mDocument Is Nothing Then
        Set mVisioApp = CreateObject(VISIO_CLASS_ID)
        If Len(mPath) <> 0 Then
            Set mDocument = mVisioApp.mDocuments.Open(mPath)
        Else
            Set mDocument = mVisioApp.mDocuments.Add
        End If
    Else
        Set mVisioApp = mDocument.Application
    End If
    Init = True
End Function

Public Property Get ScreenUpdating() As Boolean
    ScreenUpdating = mScreenUpdating
End Property

Public Property Let ScreenUpdating(ByVal ScreenUpdating As Boolean)
    mScreenUpdating = ScreenUpdating
End Property

Public Property Get WindowState() As Variant
    WindowState = mWindow.WindowState
End Property

Public Property Let WindowState(ByVal WindowState As VisWindowStates)
    mWindow.WindowState = WindowState
End Property

Public Sub setWindow()
    Set mWindow = mVisioApp.Windows(mDocument.Title)
End Sub

Change Log

• VisWindowStates added to support refactoring of the WindowState property

• Init modified to add a new Document when there is no Path specified

Answer 2

Some simple naming observations

Private Function VisioInstanceExist() As Boolean
    On Error Resume Next
    Dim visioApp As Object
    Set visioApp = GetObject(, VISIO_CLASS_ID)
    VisioInstanceExist = Not visioApp Is Nothing
End Function

Looks like a round-about way to creating an instance of Visio. It's not really a boolean at all. If you checked if it existed and returned a boolean to decide whether you need to create one, then it would be boolean.

Moreover, I don't see any way that the instance of Visio could exist at that point, it's the first thing you do and you've not set anything to the object.

I'd unrefactor this

Private Function GetVisioInstance() As Object
    Dim visioApp As Object
    On Error Resume Next
    Set visioApp = GetObject(, VISIO_CLASS_ID)
    If visioApp Is Nothing Then
        GetVisioInstance = StartVisioInstance
    Else
        Set GetVisioInstance = visioApp()
    End If
End Function

See update at bottom.

That's really not too bad of a function and it eliminates 3 others. Your call on the error checking.

Let's look at what procedures we have left (ignore that it's in excel, that shouldn't matter) _{created in RubberDuck-VBA}

Wait! There's no .Initialize() function. Maybe you can do all the creation and getting during initialization?

Otherwise -

• ActivateVisioInstance
• ActivateVisioWindow
• DetermineDesiredVisioWindow
• LoadVisioDrawing
• MaximizeVisioWindow
• OpenVisioDrawing
• SyncVisioDrawing
• VisioScreenUpdating

Now, I'll admit I'm not a Visio expert, but isn't this being run in Visio? Or, if it isn't, this Class has Visio in its name.

I think it's pretty clear we're dealing with Visio, don't you? That's just muddying the naming waters and really doesn't help to understand what is going on with each function. Why not just drop it?

• ActivateInstance
• ActivateWindow
• DetermineDesiredWindow
• LoadDrawing
• MaximizeWindow
• OpenDrawing
• SyncDrawing
• ScreenUpdating

Hm, that's better. I don't like that ScreenUpdating function, there's already Application.ScreenUpdating. Seems maybe that could just be a boolean property to Let and Get? I only actually see that property change once and then back.

Private Sub ActivateVisioInstance(ByRef app As Object)
    AppActivate app.window.Caption
End Sub

Here you're using the AppActivate statement, but you've wrapped it in a ActivateInstance method. And you're using a caption that is a property of Window (the visio object, not your variable). But I don't see where you're using that property, I don't know what the caption will be. It seems like your ActivateInstance procedure is less efficient and/or clear than if you just used the AppActivate statement in the first place. If you're going to wrap it in something, make use of that! You aren't even taking a caption argument!

SyncVisioDrawing drawing, someData
Private Sub SyncVisioDrawing(ByRef drawing As Object, ByVal someData As String)
    drawing.Title = someData
End Sub

You're passing an application object into a procedure to set its title, which you're also passing to the procedure? so it's SetDrawingTitle? Using that procedure isn't needed. If it is, name it better, take a different argument, maybe turn it into a function to get the title, or even better - make it a property of the Class.

One thing you asked about was OpenVisioDrawing being confused with LoadVisioDrawing. Yes, that's not great naming, you're right. What is OpenVisioDrawing doing?

1. Checking and creating an instance of Visio
2. Loading a drawing into Visio
3. Fixing the view of Visio to what you need

That's a lot of things, and a lot of those things are broken out to other procedures already (which is great!). I'd say the sub is managing all of the visio procedures, or being the interface to visio (that's the class name right?).

---

Naming Variables

Dim app As Object
Dim drawing As Object
Dim window As Object
Dim visioApp As Object
Const VISIO_CLASS_ID As String = "Visio.Application"
Const visOpenRW = &H20
Const visWSMaximized = &H40000000

Your first constant is UPPER_SNAKEY_CASE but the other two aren't. They aren't given Types either, so they are variants. What should they be? Strings I must assume. So make them strings.

Also, what are those values? &H20 and &H40000000 - hex notations? Of what? ASCII characters? Those constants names should at least tell me what they should be! Otherwise

Const visOpenRW = &H20
Set LoadVisioDrawing = app.Documents.OpenEx(filename, visOpenRW)

I have no idea what that should do. I don't know offhand what .OpenEx parameters are (because I'm no Visio expert). Let's find out.

What? Those are flags and it is an integer? And visOpenRW already is the constant for it. Okay, now a lot more of that makes sense, but if there's already a constant, just use it as the flag, no need to tell me there's a constant, apparently an integer, that goes as an argument (flag) into an expression. I'd know more just by

Set LoadVisioDrawing = app.Documents.OpenEx(filename, visOpenRW)

At least I won't chase my tail around for your constant's hex notation only to find out the flag visOpenRW is a built-in constant for that expression.

The variable app is what I would call vague. I see you used visioApp once and most of your functions said "visio" - why not just use visioApp or visioInstance or something similar as your variable name? That will make sure I know we're in Visio.

You've also named a variable the same name as a Visio Object - window. That can get confusing, but it's your call

Sub OpenVisioDrawing(ByVal filename As String, someData As String)
Private Function StartVisioInstance() As Object
Sub ActivateVisioInstance(ByRef app As Object)
Function LoadVisioDrawing(ByRef app As Object, ByVal filename As String) As Object
Sub SyncVisioDrawing(ByRef drawing As Object, ByVal someData As String)
Function DetermineDesiredVisioWindow(ByRef app As Object, ByRef drawing As Object) As Object
Sub ActivateVisioWindow(ByRef window As Object)
Sub MaximizeVisioWindow(ByRef window As Object)
Sub VisioScreenUpdating(ByRef app As Object, ByVal allow As Boolean)

All your functions are returning something and all of your subs are doing something, great! Your variable naming is consistent, but it's not all that descriptive.

I see a lot of ByRef in there, and several that are implicitly ByRef because they've not been designated ByVal. Why are you sending everything as a reference? You're sending Objects ByRef into Functions that return Objects.

---

Class

I'm unclear as to why this class is existing. It doesn't seem you'll really need a factory for a bunch of Visio and I don't see any properties being stored in the class. I mean your class is essentially a Visio application object, if I'm reading this correctly (and I may not be).

---

Re: other

I ran this from Access 2016 with a sample vxd and the Visio window never stopped blinking in the taskbar for a new instance of Visio. If it was already open, no blinking.

The blinking starts at

Set LoadVisioDrawing = app.Documents.OpenEx(filename, visOpenRW)

Itried moving ActivateVisioInstance to after LoadVisioDrawing but still, it blinks. I don't have a solution for that, I'm just being a negative nancy and pointing out problems :(

Stepping through the code though, still I don't understand how VisioInstanceExist can possibly be True without you calling, essentially AttachVisioInstance.

I was wrong with my example of a new version of that function. It should be

Private Function GetVisioInstance() As Object
    On Error GoTo NewInstance
    Set GetVisioInstance = GetObject(, VISIO_CLASS_ID)
    Exit Function
NewInstance:
    Set GetVisioInstance = CreateObject(VISIO_CLASS_ID)
End Function

That's my fault, sorry. The point still remains though.

OpenVisioDrawing should be named better, but as far as what to name it, I can only give suggestions. VisioHandler, VisioInterfacing, VisioController, something more broad.

Re: Class

If I move all of the class outside to the test module and just

Sub Test()
    OpenVisioDrawing "C:\Users\Ray\myVisioFile.vsd", "Custom drawing title"
End Sub

It works exactly the same as if it's in a class. Essentially your app is your object, not your class. And an instance of Visio is an object. What I mean is, why is this a class if you aren't using any special properties? I'm not saying it shouldn't be a class, I'm saying think about why it's a class and what it being a class can do for you differently than if you just stick it in a module.

Sub Test()
    Dim visioHandler As Object
    Set visioHandler = New VisioInterface
    With visioHandler
        .OpenVisioDrawing "C:\Users\Ray\myVisioFile.vsd", "Custom drawing title"
    End With
End Sub

Right, now we have an instance of the class that's an object outside of the class. What can we do with that?

Let's say in your class you put in

Private windowTitle As String
Public Property Get Title() As String
    Title = windowTitle
End Property
Public Property Let Title(ByVal newTitle As String)
    windowTitle = newTitle
End Property

Now you have something you can use outside of the class.

Sub Test()
    Const PATH As String = "C:\Users\Ray\myVisioFile.vsd"
    Dim visioHandler As Object
    Set visioHandler = New VisioInterface
    'Let .Title
    visioHandler.Title = "My Title"
    'Get .Title
    MsgBox visioHandler.Title
    'Get .Title
    visioHandler.OpenVisioDrawing PATH, visioHandler.Title
End Sub

So instead of just sending an entire thing into the class to do everything, you can use those methods in the standard module.