"Cary Shultz" <cshultz@xxxxxx> wrote in message
news:eUox20N7JHA.2656@xxxxxx
Quote:
> Good morning, All!
>
> Okay, I am learning VBScripting and have several scripts (about 15 or so).
> Most of them look like the one below. I just would like to ask those of
> your with far more experience than I have to take a look at this and let
> me know if there is anything that is not correct or anything that could be
> done better. We are running these scripts in production environments
> (after some pretty stringent testing....). To my knowledge there is
> nothing wrong with this script...it runs and produces the desired results.
> I would just like to know if it can be optimized. I have run it without
> the "On Error Resume Next" without issue.....All of that was done during
> the testing.
>
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>
>
> '==================================================================================================
> '
> ' VBScript Source File
> '
> ' NAME: #Fixed-Drives.VBS
> ' VERSION: 1.0
> ' AUTHOR: xxxxxxxxxx
> ' COMPANY: xxxxxxxxxx
> ' CREATE DATE : 06/03/2009
> ' LAST MODIFIED : n/a
> '==================================================================================================
> ' COMMENT: This script will list all Drives on a Computer
> '==================================================================================================
>
> Option Explicit
> On Error Resume Next
>
> Dim objWMIService, colFixedDrives, objFD
> Dim objFSO, objTextFile, objTS
> Dim strComputer
> Dim strInputFile, strOutputFile
>
> Const ForReading = 1
> Const ForWriting = 2
>
> strInputFile = "C:\Scripts\Servers Info\Servers-TxtFile.txt" 'this
> file list the servers against which this specific script is to run....the
> servers are listed one line at a time
> strOutputFile = "C:\Scripts\Results\Fixed-Drives.txt"
>
> Set objFSO = CreateObject("Scripting.FileSystemObject")
> Set objTextFile = objFSO.OpenTextFile(strInputFile,ForReading)
> Set objTS = objFSO.CreateTextFile (strOutputFile,ForWriting,False)
>
> Do Until objTextFile.AtEndOfStream
> strcomputer = objTextFile.Readline
>
> Set objWMIService = GetObject("winmgmts:" _
> & "{impersonationLevel=impersonate,authenticationLevel=Pkt}!\\" &
> strComputer & "\root\cimv2")
>
> If (Err.Number <> 0) Then
> objTS.WriteLine()
> objTS.WriteLine "ERROR: Unable to connect to the WMI NameSpace on " &
> strComputer
> objTS.WriteLine()
> objTS.WriteLine
> "......................................................................"
> Err.Clear
> Else
>
> Set colFixedDrives = objWMIService.ExecQuery ("Select * from
> Win32_LogicalDisk WHERE DRIVETYPE = 3",,48)
>
> For Each objFD in colFixedDrives
> objTS.WriteLine "Computer Name: " & strComputer
> objTS.WriteLine "Device ID: " & objFD.DeviceID
> objTS.WriteLine "Drive Type: " & objFD.DriveType
> objTS.WriteLine "Free Space: " &
> ROUND(objFD.FreeSpace/1073741824,2) & " GB"
> objTS.WriteLine "Size: " &
> ROUND(objFD.Size/1073741824,2) & " GB"
> objTS.WriteLine "Volume Name: " & objFD.VolumeName
> objTS.WriteLine
> "......................................................................"
> Next
>
> End If
>
> LOOP
>
> objTextFile.Close
> objTS.Close
>
> Set objTextFile = Nothing
> Set objTS = Nothing
> Set objFSO = Nothing
>
>
>
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>
>
> Thanks everyone!
Here are a few comments:
- Having a global "on error resume next" is not a good idea because it makes
trouble-shooting very difficult. It basically means "When you find an error,
just pretend everything is OK and continue with whatever you were doing.".
Much better to use the statement only when absolutely required and to turn
it off immediately below the spot to be monitored.
- The statement objTS.WriteLine "....................." could be written
more elegantly as objTS.WriteLine string(30, ".").
Other than this the script looks fine to me - keep up the good work!