Pegasus [MVP] wrote:
Quote:
>
> "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! I will have to respectfully disagree regarding the comments about On
Error Resume Next.
On Error Resume Next is essential if you wish to make an enterprise
script provide valuable troubleshooting information.
I am in total agreement that it should not be used to just ignore
errors, however it needs to be used to identify error numbers and error
descriptions. Furthermore, if I am running a script against multiple
servers, I want to generate a report of problem servers and keep moving
on to the next server. To have a script stop execution because one out
of a hundred servers is not available would be counter productive. The
very purpose of the script might be to determine what server are not
available.
Cary, you have mentioned you want to add a check to see if a server is
available, have a look at my FAQ
http://www.tek-tips.com/faqs.cfm?fid=4871 and grab the PingStatus
function. You will note that the PingStatus starts with an On Error
Resume Next. If a server is unavailable you will error out in trying to
establish a WMI connection to it, so again I would argue that in many
cases On Error Resume Next is an essential tool, but one that needs to
be used in the right context.
--