HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Check if an XML file exists then change URLs in it

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filexmlthenexistscheckchangeurls

Problem

This is my first VBScript I have ever written and I would like to know if there is any obvious blunders in the code. All the code should do is check if file exists then change the Yahoo URL to the Google URL in the XML file. Note: The code seems to function properly.

Dim objFSO
Set objFSO = CreateObject("Scripting.FileSystemObject")

If (objFSO.FileExists("C:\urlsinfo.xml")) Then
Const ForReading = 1
Const ForWriting = 2
Const ForAppending = 8

Dim textToReplace
Dim textReplacement
textToReplace = "http://www.yahoo.com"
textReplacement = "http://www.google.com"

Set objFile = objFSO.OpenTextFile("C:\urlsinfo.xml", ForReading)
strText = objFile.ReadAll
objFile.Close
strNewText = Replace(strText, textToReplace, textReplacement)

Set objFile = objFSO.OpenTextFile("C:\urlsinfo.xml", ForWriting)
objFile.WriteLine strNewText
objFile.Close
WScript.Quit()

Else
Dim errorFileSys, errorLogTxt

Set errorFileSys = CreateObject("Scripting.FileSystemObject")
Set errorLogTxt = errorFileSys.OpenTextFile("C:\urlsinfoError.log", ForAppending, True) 
errorLogTxt.WriteLine(Now &"     urlsinfo.xml file did not exist in directory C:\. ")
errorLogTxt.Close

WScript.Quit()
End If

'Exit Script
WScript.Quit()

Solution

I reworked your VBScript a bit with comments, see below. In response to the comment from Konrad, I can agree with him for 90% of the cases. VBScript stays interesting for system management, logon scripts, etc., but I myself am replacing it with Ruby which isn't a big step up from VBScript, and on the other hand it has almost all the goodies from python. If you want to compare, take a look at e.g. http://yagni.com/rosetta-stone/#0 (what happened to https://rosettacode.org/ ?). You'll see that the Ruby code is most of the time the smallest and the most comprehensible.

The adapted VBScript version:

Option explicit 'always use this
'declarations first, be concise
Const ForReading = 1, ForWriting = 2, ForAppending = 8, CreateIfNeeded = true
Dim objFSO, objFile, textToReplace, textReplacement, strNewText, strText
Dim errorFileSys, errorLogTxt, urlsinfo
Set objFSO = CreateObject("Scripting.FileSystemObject")

If objFSO.FileExists(urlsinfo) Then 'indent
  textToReplace   = "http://www.yahoo.com"
  textReplacement = "http://www.google.com"
  urlsinfo = "C:\urlsinfo.xml"
  Set objFile = objFSO.OpenTextFile(urlsinfo, ForReading)
  strText = objFile.ReadAll
  objFile.Close
  strNewText = Replace(strText, textToReplace, textReplacement)
  Set objFile = objFSO.OpenTextFile(urlsinfo, ForWriting)
  objFile.WriteLine strNewText
  objFile.Close
Else
  Set errorFileSys = CreateObject("Scripting.FileSystemObject")
  Set errorLogTxt = errorFileSys.OpenTextFile("C:\urlsinfoError.log", ForWriting, CreateIfNeeded) 
  errorLogTxt.WriteLine(Now &"     urlsinfo.xml file did not exist in directory C:\. ")
  errorLogTxt.Close
  Wscript.Quit 1 'you could use quit here to give an errorlevel back to the OS
End If

'Exit Script 'no need for obvious comments
'WScript.Quit() 'no need for this at the end of the script
'also quit is a sub, no need for the ()


And the same script in Ruby:

urlsinfo, urlsinfoError = "C:/urlsinfo.xml", "c:/urlsinfoError.log"
textToReplace, textReplacement = "http://www.yahoo.com", "http://www.google.com"

begin
  File.write(urlsinfo, File.read(urlsinfo).gsub(textToReplace, textReplacement))
rescue
  File.write(urlsinfoError, "#{Time.now}      urlsinfo.xml file did not exist in directory C:\. ")
end

Code Snippets

Option explicit 'always use this
'declarations first, be concise
Const ForReading = 1, ForWriting = 2, ForAppending = 8, CreateIfNeeded = true
Dim objFSO, objFile, textToReplace, textReplacement, strNewText, strText
Dim errorFileSys, errorLogTxt, urlsinfo
Set objFSO = CreateObject("Scripting.FileSystemObject")

If objFSO.FileExists(urlsinfo) Then 'indent
  textToReplace   = "http://www.yahoo.com"
  textReplacement = "http://www.google.com"
  urlsinfo = "C:\urlsinfo.xml"
  Set objFile = objFSO.OpenTextFile(urlsinfo, ForReading)
  strText = objFile.ReadAll
  objFile.Close
  strNewText = Replace(strText, textToReplace, textReplacement)
  Set objFile = objFSO.OpenTextFile(urlsinfo, ForWriting)
  objFile.WriteLine strNewText
  objFile.Close
Else
  Set errorFileSys = CreateObject("Scripting.FileSystemObject")
  Set errorLogTxt = errorFileSys.OpenTextFile("C:\urlsinfoError.log", ForWriting, CreateIfNeeded) 
  errorLogTxt.WriteLine(Now &"     urlsinfo.xml file did not exist in directory C:\. ")
  errorLogTxt.Close
  Wscript.Quit 1 'you could use quit here to give an errorlevel back to the OS
End If

'Exit Script 'no need for obvious comments
'WScript.Quit() 'no need for this at the end of the script
'also quit is a sub, no need for the ()
urlsinfo, urlsinfoError = "C:/urlsinfo.xml", "c:/urlsinfoError.log"
textToReplace, textReplacement = "http://www.yahoo.com", "http://www.google.com"

begin
  File.write(urlsinfo, File.read(urlsinfo).gsub(textToReplace, textReplacement))
rescue
  File.write(urlsinfoError, "#{Time.now}      urlsinfo.xml file did not exist in directory C:\. ")
end

Context

StackExchange Code Review Q#12439, answer score: 6

Revisions (0)

No revisions yet.