# Checking and updating lines in a file

I am trying to check a bunch of files that should have a leading | delimiter but sometimes don't. If the first line doesn't have the |, the rest of the file won't, if it does have it, then the rest of the file will.

I'm checking the first line which works and enters the update logic, which advances to the next row once I read the line in. So I added a writer.Write before I go into the Do While loop and it works, but it seems like there would be a better way of doing this.

    Dim fileList As FileInfo() = dirList.GetFiles

For Each fiName In fileList
sFullName = fiName.FullName
sFileName = Path.GetFileNameWithoutExtension(fiName.ToString)
sDir = fiName.DirectoryName

If Not line.StartsWith("|") Then

Dim sNewFullName As String = sDir + "\" + sFileName + "_Temp.txt"
Dim writer As New StreamWriter(sNewFullName)
Dim sLine As String

sLine = "|" + line + System.Environment.NewLine
writer.Write(sLine)

Do While reader.Peek() <> -1
sLine = "|" + reader.ReadLine() + System.Environment.NewLine
writer.Write(sLine)
Loop

writer.Close()
File.Delete(sFullName)
File.Move(sNewFullName, sFullName)

End If
Next fiName


As Mat's Mug already pointed out, your variable naming should be enhanced.

So

Dim sNewFullName As String = sDir + "\" + sFileName + "_Temp.txt"


would become

Dim newFullName As String = System.IO.Path.Combine(currentDirectoryName, currentFileName + "_Temp.txt")


As Mat's Mug also pointed out you need to extract some of the responsibilities to separate methods.

So let us introduce a method which takes a filename as input parameter and returns an array of Strings representing the content of the file.

Private Function GetFileContent(fileName As String) As String()
End Function


next we introduce a function which takes a filename as input parameter and returns wether the first character of the first line is the "|" . We first check if the read line is maybe Nothing by using String.IsNullOrWhiteSpace(). This prevents an exception if the file would be empty.

Private Function FirstLineStartsWithPrefix(fileName As String, prefix As String) As Boolean
Return Not String.IsNullOrWhiteSpace(line) AndAlso line.StartsWith(prefix)
End Using
End Function


Next we introduce a method which takes a String array as input parameter and returns the prefixed content. We are using the Append() and AppendLine() method. Both methods are returning a StringBuilder object, so we can use it in a fluent way.

Private Function AddPrefixToLines(content As String(), prefix As String) As String
Dim sb As New System.Text.StringBuilder(1024)
For Each line As String In content
sb.Append(prefix).AppendLine(line)
Next
Return sb.ToString()
End Function


Next we introduce a method which takes a FileInfo object as input parameter and returns nothing (a Sub).

Private Const pipePrefix As String = "|"
Private Const fileNamePostFix As String = "_Temp.txt"
Private Sub ProcessFile(currentFileInfo As FileInfo)

Dim fullName As String = currentFileInfo.FullName

If Not FirstLineStartsWithPrefix(fullName, pipePrefix) Then Exit Sub

Dim fileContent As String() = GetFileContent(currentFileInfo.FullName)

Dim prefixedContent As String = AddPrefixToLines(fileContent, pipePrefix)

Dim pureFileName As String = Path.GetFileNameWithoutExtension(fullName)

Dim newFileName As String = Path.Combine(currentFileInfo.DirectoryName, pureFileName & fileNamePostFix)

File.WriteAllText(newFileName, prefixedContent)

currentFileInfo.Delete()

File.Move(newFileName, fullName)

End Sub


Now your initial code will look like

Dim fileList As FileInfo() = dirList.GetFiles

For Each fiName In fileList

ProcessFile fiName

Next

• Got sidetracked with other projects but returning to this finally. For your suggestion on the method to read all lines into an array, I switched the StringReader to a StreamReader because at that point the file hasn't been read into the array. I also broke up the FirstLineStartsWithPrefix function because it was returning false: Dim fileEmpty As Boolean = String.IsNullOrWhiteSpace(line) If fileEmpty = True Then Return False Else Return Not line.StartsWith(prefix) End If – Dave Jan 2 '15 at 19:31

Both StreamReader and StreamWriter classes inherit the Dispose method from their base class, which implements the IDisposable interface; you should always dispose of disposables - and doing so makes calling the Close method redundant. It's good that you're closing your streamreader/writer, but it's better to dispose them.

VB has a Using statement specifically for that purpose, so you don't even need to explicitly call Dispose yourself - see MSDN.

Your code has too many responsibilities. I would extract a method out of the loop's body.

The names shouldn't be prefixed with that s or any other prefix that stands for the identifier's type - that's Hungarian Notation, and heavily discouraged in any .net language.

I like that you are using the .net constants and methods instead of the legacy (/VB6) ones in the Microsoft.VisualBasic namespace.