Vista - Look over this script.....could anything be done better?

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!

"Cary Shultz" <cshultz@xxxxxx> wrote in message
news:eUox20N7JHA.2656@xxxxxxHere 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!

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

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

Richard,

All excellent points......especially on the "even if ping works....". I
have run into corrupted WMI on specific machines several times! Always fun!

That will be revision 4 for these scripts......I need to learn how to do
that better than what I (think I) know how to do now....

BTW - that might be a good 'section' for your web site: error handling!

Thanks,

Cary


"Richard Mueller [MVP]" <rlmueller-nospam@xxxxxx> wrote in
message news:uF6fQTQ7JHA.1716@xxxxxx

Pegasus [MVP] wrote:
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.

--