View Single Post
  #2  
Old 08-03-2021, 12:23 PM
gregg gregg is offline
Registered User
 
Join Date: Oct 2010
Posts: 75
Hiya Rhudi-

Quote:
We all need a reliable way to paste commands into a router. Really, what else does any of use us SecreCRT for anyway?
haha- i may or maynot work for Cisco, and i rarely if ever connect to networking devices.

Anyway, my critiques are these.

The code presented is too monolithic and should really be broken down into easily digestible parts, aka more subs/functions with good names.

The indentation is too deep which makes code really difficult to follow.

Code:
    Select Case crt.Session.Connected
        Case False
            crt.Dialog.MessageBox "Sending data requires an active connection."
        Case True
can just become

Code:
If crt.Session.Connected = False Then
    crt.Dialog.MessageBox "Sending data requires an active connection."
    Exit Sub
End If

If Trim(crt.Clipboard.Text) = "" Then
    crt.Dialog.MessageBox "No text found in the Windows Clipboard."
    Exit Sub
End If
That immediately kills multiple levels of indentation. Just get out early if nothing else to do.

As far as breaking parts down into Subs and Functions, here's an example.

Code:
                        Case "-re0>", "-re1>", "-re0#", "-re1#"
                            sPrompt1 = Right(sPrompt1, 5) ' "-re0>" or "-re1>"
                            objTab.Screen.Send "set cli screen-length 0" & vbCr
                            sConnectionType = "JUNIPER"
                            bJunosConnected = True
                            bCiscoConnected = False
Something like this, I would just:

Code:
Function InitJuniperConnection()
    sPrompt1 = Right(sPrompt1, 5) ' "-re0>" or "-re1>"
    objTab.Screen.Send "set cli screen-length 0" & vbCr
    sConnectionType = "JUNIPER"
    bJunosConnected = True
    bCiscoConnected = False
    InitJuniperConnection = sPrompt1
End Function
...
        Case "-re0>", "-re1>", "-re0#", "-re1#"
            sPrompt1 = InitJuniperConnection
Same for Cisco. Though I wonder why you need 3 variables to indicate the connection type. I guess the booleans are unused, but if you needed them you could:

Code:
Dim sConnectionType
sConnectionType = "UNKNOWN"

Function isJuniper()
    ' in a sane language, this would be written as
    ' return sConnectionType == "JUNIPER"
    ' to distinguish assignment from comparison that returns a bool
    isJuniper = sConnectionType = "JUNIPER"
End Function

Function isCisco()
    isCisco = sConnectionType = "CISCO"
End Function

' and to use one:
If isJuniper() Then
    ' juniper specific code
End If
Anyway, those are some of my immediate thoughts on how i would start the refactor.
Reply With Quote