View Single Post
Old 06-14-2009   #4 (permalink)
Richard Mueller [MVP]


 
 

Re: Look over this script.....could anything be done better?

Even when a computer responds to the ping, there is the chance that you will
be unable to connect with WMI (although this is getting less likely). The
remote computer might not have WMI installed, or WMI might be corrupt, or
the class you are using might not be supported. In scripts where I ping
before attempting to connect with WMI, I still trap the possible error. But
I only use "On Error Resume Next" for the one statement that might raise the
error. I test with Err.Number to see if an error was raised, handle the
error, and restore normal error handling immediately. Otherwise bad things
can happen. And if there is an unexpected problem, I always want to know.

--
Richard Mueller
MVP Directory Services
Hilltop Lab - http://www.rlmueller.net
--

"Cary Shultz" <cshultz@xxxxxx> wrote in message
news:exv6$qP7JHA.1716@xxxxxx
Quote:

> Pegasus~
>
> Thanks for the feedback and the tip (objTS.....). I learn something
> everytime I post....you will be seeing more and more of me....
>
> Agreed on the On Error Resume Next. With this version of the script (one
> more revision coming) I have kept that included because the script that
> enumerates all of the servers (essentially queries AD and returns the list
> of servers - without checking to see if there is actually a computer
> associated with the object) has some servers that no longer exist (working
> on cleaning up AD...using Joe's tools for that).
>
> Naturally, you know the next line from me.......without the "On Error
> Resume Next" the script stops with the "object not found" at the GetObject
> part of the script. In the testing environment that was not a problem.
> When I tested in an environment that I built and maintain there was no
> issue. However, when I ran it in a new client (where I have not run Joe's
> tools yet) there were two objects for servers that do not exist
> anymore.....STOP!!!!. So, for the moment I simply keep the "On Error
> Resume Next". I hate it but for the time being it will stay...
>
> I just put in the "If (Err.Number <> 0) Then" part and that made the
> output file so much nicer! So, that was my 2nd revision.
>
> The next thing is going to be adding comspec...ping.exe to actually ping
> the servers as provided by that "input" file. Once I get that part down
> then the "On Error Resume Next" part comes out! Good ridance!
>
> Thanks again,
>
> Cary
>
>
> "Pegasus [MVP]" <news@xxxxxx> wrote in message
> news:edcvL9O7JHA.3860@xxxxxx
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!
>>
>

My System SpecsSystem Spec