Troubleshooting File Copying and Removing Items in Visual Basic Function

[phion]

Hi,

I'm having trouble figuring out how to copy the contents of a file over to another file, and removing an item of the content of the list I'm copying in the process. The idea is that I need to remove an item from a list by copying everything that is in file one over to file two that is not the item I need to remove, deleting the original file, and the renaming the second file to the original. I can't seem to get it to work.

Here's the function that main calls.

Function removePlayer(playerList() As String, numPlayers As Integer) As Integer
        Dim updatedPlayers As Integer
        Dim player As String
        Dim fileName As String = "playerList.txt"
        Dim tempFileName As String = "tempPlayerList.txt"
        Dim inFile As New StreamReader(fileName)
        Dim outFile As New StreamWriter(tempFileName)

        Console.WriteLine("Which player would you like to remove?")

        player = Console.ReadLine()

        While Not inFile.EndOfStream

            For i = 0 To numPlayers - 1

                playerList(i) = inFile.ReadLine()

                If playerList(i) <> player Then

                    outFile.WriteLine(playerList(i))

                End If

            Next

        End While

        inFile.Close()
        outFile.Close()

        File.Delete("playerList.txt")

        My.Computer.FileSystem.RenameFile("C:\Users\John\Desktop\addPLayerList\bin\Debug\tempPlayerList.txt", "playerList.txt")

        File.Create("C:\Users\John\Desktop\addPLayerList\bin\Debug\tempPlayerList.txt")

        updatedPlayers = numPlayers - 1

        Return updatedPlayers
    End Function

 

[phinds]

"Doesn't work" is a useless characterization of what you are experiencing. OK, it doesn't do what you want. Fine. What DOES it do? What are the symptoms of the error?

 

[Andrew Mason]

The problem may be with the use of <> for strings.

Rather than use "If playerList(i) <> player Then" you might use:

x = Instr(playerList(i), player)
if x = 0 then
outFile.WriteLine(playerList(i))
endif

You might also want to ensure that all strings are put to the same case (ucs or lcs) when comparing.

AM

 

[phion]

"Doesn't work" is a useless characterization of what you are experiencing. OK, it doesn't do what you want. Fine. What DOES it do? What are the symptoms of the error?

It doesn't effectively delete an item from the list, but what it does do is copy the whole list to a temporary file, delete the original, and rename the temporary file to the original name. I need it to leave out the item to be deleted when it copies the list over to the new file.

 

[phinds]

It doesn't effectively delete an item from the list, but what it does do is copy the whole list to a temporary file, delete the original, and rename the temporary file to the original name. I need it to leave out the item to be deleted when it copies the list over to the new file.

Then clearly, your test "playerList(i) <> player" isn't doing what you want. Are you sure you are typing in EXACTLY what is in the line you want to get rid of? Case matters.

 

[phion]

Then clearly, your test "playerList(i) <> player" isn't doing what you want. Are you sure you are typing in EXACTLY what is in the line you want to get rid of? Case matters.

Now what it's doing is deleting the last item in the list regardless of what name I specify to be deleted. Strange.

 

[phinds]

Now what it's doing is deleting the last item in the list regardless of what name I specify to be deleted. Strange.

Are you saying that it is now deleting the one you want AND the last one or just the last one? PLEASE be exact when you are describing these conditions. Trying to guess what you mean is annoying.

 

[phion]

Are you saying that it is now deleting the one you want AND the last one or just the last one? PLEASE be exact when you are describing these conditions. Trying to guess what you mean is annoying.

It's only deleting the last item in the list, no matter what item I want deleted.

 

[phinds]

Sounds like you are passing a number that tells it to read one less than you want it to read plus whatever is causing it to not delete the one you type in.

 

[Andrew Mason]

If you could provide us with a sample of playerList.txt we might have a better idea.

It is deleting the last line because you are starting at 0 instead of 1. Try starting at 1.

To figure out why it is treating as true: playerList(i) <> player when playerList(i) = player you just have to display these variables and see what it is doing. Just insert a msgbox line eg:

For i = 1 To numPlayers

playerList(i) = inFile.ReadLine()
msgbox("playerList(" & i ") = " & playerList(i) & " ; player = " & player)

If playerList(i) <> player Then
msgbox("Printing playerList(" & i ") = " playerList(i))
outFile.WriteLine(playerList(i))

End If​

Next​

Also: it is not clear how you determine numPlayers. Why not just let the for loop continue until you reach the end of the infile?

AM

 

[phion]

playerList.txt displays a simple list of names that the user has input.

John
Tim
Sara
Alex

Then the program will ask, if the user chooses, which name they would like to remove from the list. If I choose, say, John to be removed the list becomes

John
Tim
Sara

When it should be

Tim
Sara
Alex

 

[phion]

Also: it is not clear how you determine numPlayers. Why not just let the for loop continue until you reach the end of the infile?

numPlayers is assigned in a different function where the user adds the players to the list.

I tried just using the for loop and what happens is that every name on the list gets replaced with the name I want to remove.

 

[Andrew Mason]

playerList.txt displays a simple list of names that the user has input.

John
Tim
Sara
Alex

In addition to starting at i=1, You should make sure the comparison is case indifferent. You should also make sure there are no leading or trailing spaces:

dim uplayer as string
dim uplayerlist as string
...
uplayer = UCase(player)
uplayer=Trim(uplayer)

For i = 1 To numPlayers

playerList(i) = inFile.ReadLine()
uplayerlist= UCase(playerList(i))
uplayerlist = Trim(uplayerlist)

If uplayerlist <> uplayer Then
outFile.WriteLine(playerList(i))

End If

Next​

Again, you should make sure that the number of lines in the input file does not exceed numPlayers. To avoid a problem with that you can just keep reading lines until you reach the end of file.

AM

 

[phion]

It's still not doing what I need it to. Here's what I have, it's a small part of a bigger program.

Imports System.IO

Module Module1

    Sub Main()
        Const SIZE As Integer = 50
        Dim playerList(SIZE), tempPlayerList(SIZE) As String
        Dim numPlayers As Integer

        numPlayers = loadList(playerList)

        numPlayers = addPlayers(playerList, numPlayers)

        displayList(playerList, numPlayers)

        numPlayers = removePlayer(playerList, numPlayers)

        displayList(playerList, numPlayers)

        saveList(playerList, numPlayers)

        Console.WriteLine("List saved successfully!")

    End Sub

    Function loadList(playerList() As String) As Integer
        Dim numPlayers As Integer
        Dim fileName As String = "playerList.txt"

        numPlayers = 0

        If File.Exists(fileName) Then

            Dim inFile As New StreamReader(fileName)

            While Not inFile.EndOfStream

                playerList(numPlayers) = inFile.ReadLine()

                numPlayers += 1

            End While

            inFile.Close()

        Else
            Console.WriteLine("Could Not Open " & fileName)
        End If

        Return numPlayers
    End Function

    Function addPlayers(playerList() As String, numPlayers As Integer) As Integer
        Dim newPlayer As String

        Console.Write("Enter player name ('exit' to quit): ")

        newPlayer = Console.ReadLine()

        While newPlayer <> "exit"

            playerList(numPlayers) = newPlayer

            numPlayers += 1

            Console.Write("Enter player name ('exit' to quit): ")

            newPlayer = Console.ReadLine()

        End While

        Return numPlayers

    End Function

    Sub displayList(playerList() As String, numPlayers As Integer)
        Dim i As Integer

        For i = 0 To numPlayers - 1

            Console.WriteLine("Player " & i + 1 & ": " & playerList(i))

        Next

    End Sub

    Function removePlayer(playerList() As String, numPlayers As Integer) As Integer
        Dim updatedPlayers As Integer
        Dim player As String
        Dim fileName As String = "playerList.txt"
        Dim tempFileName As String = "tempPlayerList.txt"
        Dim inFile As New StreamReader(fileName)
        Dim outFile As New StreamWriter(tempFileName)

        Console.WriteLine("Which player would you like to remove?")

        player = Console.ReadLine()

        For i = 1 To numPlayers

            playerList(i) = inFile.ReadLine()

            If player <> playerList(i) Then

                outFile.WriteLine(playerList(i))

            End If

        Next

        inFile.Close()
        outFile.Close()

        File.Delete("playerList.txt")

        My.Computer.FileSystem.RenameFile("C:\Users\John\Desktop\addPLayerList\bin\Debug\tempPlayerList.txt", "playerList.txt")

        File.Create("C:\Users\John\Desktop\addPLayerList\bin\Debug\tempPlayerList.txt")

        updatedPlayers = numPlayers - 1

        Return updatedPlayers
    End Function

    Sub saveList(playerList() As String, numPlayers As Integer)
        Dim fileName As String = "playerList.txt"
        Dim outFile As New StreamWriter(fileName)

        For i = 0 To numPlayers - 1

            outFile.WriteLine(playerList(i))

        Next

        outFile.Close()
    End Sub

End Module

This code works just as I need it to, except when I try to remove a player. I tried taking out the while loop and only using a for loop, but it still just replaces every name with the name I want to remove. I'm careful to use the right cases so I'm not too worried about that part just yet.

 

[mfb]

I don't get the logic:
loadList reads in a file and puts the player list into an array "playerList".
addPlayers modifies this array: it adds an element.
displayList prints out the array.
saveList saves the array. All consistent so far.
removePlayer does not care about the existing array at all, it reads the file again and processes it.

Your array seems to begin at zero (see saveList and displayList), so you should start at zero in your loop.

This code (your last post) replaces entries with the input? Then something odd happens with outFile.WriteLine(playerList(i)). Did you check what is stored at playerList(i) at that point?
What happens if you use a player name (to be removed) not present in the list?
Does saveList work properly?

 

[phion]

I don't get the logic:
loadList reads in a file and puts the player list into an array "playerList".
addPlayers modifies this array: it adds an element.
displayList prints out the array.
saveList saves the array. All consistent so far.
removePlayer does not care about the existing array at all, it reads the file again and processes it.

Your array seems to begin at zero (see saveList and displayList), so you should start at zero in your loop.

This code (your last post) replaces entries with the input? Then something odd happens with outFile.WriteLine(playerList(i)). Did you check what is stored at playerList(i) at that point?
What happens if you use a player name (to be removed) not present in the list?
Does saveList work properly?

Save list works fine. Using a name not on the list actually causes every name except the first to be removed. This is weird.

I set the starting index back to 0 and it's just deleting everything after the first name again. I simply don't understand.

 

[mfb]

Okay, your outFile.WriteLine() does something weird.
What happens if you replace it by outFile.WriteLine("Test")?

 

[Andrew Mason]

While Not inFile.EndOfStream

playerList(numPlayers) = inFile.ReadLine()

numPlayers += 1

End While

I thought numPlayers was a fixed number ie. that does not change. What you appear to be trying to do here is use it as an index.

I am not familiar with the numPlayers += 1 statement. Is it supposed to increment by 1 every iteration, starting at 1? If it does not do that the index will be wrong.

AM

 

[Samy_A]

I have to agree with @mfb , the logic of the program is strange.

    Sub Main()
        Const SIZE As Integer = 50
        Dim playerList(SIZE), tempPlayerList(SIZE) As String
        Dim numPlayers As Integer

        numPlayers = loadList(playerList)

        numPlayers = addPlayers(playerList, numPlayers)

        displayList(playerList, numPlayers)

        numPlayers = removePlayer(playerList, numPlayers)

        displayList(playerList, numPlayers)

        saveList(playerList, numPlayers)

        Console.WriteLine("List saved successfully!")

    End Sub

What your code apparently does:

1. You load a file into the array.
2. You let users add names to that array.
3. You display the array.
4. In removePlayer, you let the user enter a name and then you reload the file (hereby discarding the names added in step 2), fill the array with all the names in the file (including the name the user wants to remove), and save the file with all the names except the one your user chose to remove. However, you loop from 0 to numPlayers-1, where numPlayers is the number of names in the array before you called removePlayer, not necessarily the number of names in the file.
   Then you subtract 1 from numPlayers, irrespective of whether the name to be removed was present in the file or not.
   If I read it correctly, after executing removePlayer, numPlayers wll be one less than the number of names in the array before you called removePlayer.
5. You display the array.
6. You save the array.

These apparent inconsistencies don't explain the results that you get, but certainly don't make debugging this program easier.
It would also help if you could tell us where you see the weird results: is it when you display the array in step 5, or is when you look at the file?

 

[phion]

Since the index of the first element in an array is zero, the for loop starts at zero because that corresponds to the first name in the array.

 

[phion]

It would also help if you could tell us where you see the weird results: is it when you display the array in step 5, or is when you look at the file?

The weird results are apparent when the list is displayed for the second time, after the loop in the removePlayer function has run. When I actually open the file, it contains the list that was displayed the second time, which gets saved successfully. This is a skeleton that is part of a console game I'm writing, and I can't continue with it really until I figure out this strange problem.

Should I turn the arrays into objects? Use a different kind of loop? Discard the text file and only use arrays (which seems even more difficult)?

 

[phion]

Could someone just write a sample piece of code that can do what I need?

 

[Samy_A]

Could someone just write a sample piece of code that can do what I need?

That's not evident as we don't know exactly what the function must do.

Try to separate concerns.
You have a method to read a file into the array, you have a method to save the array into a file.
So don't repeat this in the removePlayer function: call these methods when needed.

If you must read the file in removePlayer, use

While Not inFile.EndOfStream

and not

For i = 0 To numPlayers - 1

, if only because you don't know the number of players while reading the file.

Also address the evident mistakes in removePlayer (as writing all players to the array, including the removed one, or the inconsistencies with the numPlayer counter).
And even if you think that it is not necessary, apply what @Andrew Mason suggested:

You should make sure the comparison is case indifferent. You should also make sure there are no leading or trailing spaces

 

[Andrew Mason]

Do what Samy_A has said. And while you are at try displaying the variables that are being compared (only while you are debugging it - take them out when it is working properly). That is the simplest way to see what the program is doing.

AM

 

[jtbell]

And while you are at try displaying the variables that are being compared (only while you are debugging it - take them out when it is working properly). That is the simplest way to see what the program is doing.

Yes, if you're puzzled by the results of a program not being what you want it to do, don't waste too much time simply staring at the code and trying to guess what it's actually doing. Add some "debug print" statements (or use a debugging tool) so that it tells you what it's actually doing.

 

[phion]

I will have to bone up on my debugging skills. I've stared at this thing for days. I guess what I am hoping for is someone to show me another way to do what I need, possibly using a different syntax or method. It's such a simple task, I don't understand.

 

[Samy_A]

I will have to bone up on my debugging skills. I've stared at this thing for days. I guess what I am hoping for is someone to show me another way to do what I need, possibly using a different syntax or method. It's such a simple task, I don't understand.

We can also stare at this code for days. But you can actually execute it.

So adapt/correct the code as suggested above, and, as also suggested above, add debug.print's (or some other way for actually seeing the value of a variable at a specific point in code execution) when what happens is not what you hoped for.

Don't assume too much, as in: "the mistake can't be here, because that piece of code looks alright". Many bugs you wilI have to deal with will be very trivial and at unexpected locations (at least that is my experience in 30+ years dealing with code).

 

[phion]

(or use a debugging tool)

What's a good debugging tool?

 

[phion]

We can also stare at this code for days. But you can actually execute it.

So adapt/correct the code as suggested above, and, as also suggested above, add debug.print's (or some other way for actually seeing the value of a variable at a specific point in code execution) when what happens is not what you hoped for.

Don't assume too much, as in: "the mistake can't be here, because that piece of code looks alright". Many bugs you wilI have to deal with will be very trivial and at unexpected locations (at least that is my experience in 30+ years dealing with code).

I've only been coding for a few months. While I do enjoy it, my professor dampens my motivation with ruthless grading practices. Thankfully programming is not my major. I didn't even want to learn VB. I was expecting him to teach C++, but whatever.

I'm not even sure where to look for VB debugging tutorials. Any advice?

 

[jtbell]

What's a good debugging tool?

I've never used Visual Basic, so I have no idea. All my programming during the past 25 years or so has been on Unix(ish) systems. I've used gdb occasionally, but normally I use plain old "debug print" statements. Also, I try to develop my programs incrementally, adding one smallish chunk of code at a time and testing it before adding the next chunk.

 

[Samy_A]

I've only been coding for a few months. While I do enjoy it, my professor dampens my motivation with ruthless grading practices. Thankfully programming is not my major. I didn't even want to learn VB. I was expecting him to teach C++, but whatever.

I'm not even sure where to look for VB debugging tutorials. Any advice?

Advice: what @jtbell said. For your problem debug.print or Console.WriteLine should be sufficient. There are more sophisticated debugging tools in Visual Studio, but I don't have any experience with them.

 

[Andrew Mason]

It looks like you are using Visual Studio.net. I am familiar with VB6. The syntax may vary but the concepts are the same. In VB6 the statement: msgbox("caption", datavariablename1, "; caption2: " & datavariablename2 ...etc. ) will stop execution and bring up a message box with the caption and data as it exists at that point in the program. Press "OK" to continue. You will be able to see what is going on. Just comment it out when you figure out what is wrong and get it running.

Is there a reason to dimension an array here? If all you are doing is copying a file and checking it line by line, you don't need to dimension an array. Just do it line by line: input a line from the source file, check it and decide whether to print it and discard or print it to the output file; then get the next line from the source file; continue until end of the source file (use a do/while loop as has been suggested). It is much simpler and a more general way to do it. Just a suggestion.

AM

 

[phion]

It looks like you are using Visual Studio.net. I am familiar with VB6. The syntax may vary but the concepts are the same. In VB6 the statement: msgbox("caption", datavariablename1, "; caption2: " & datavariablename2 ...etc. ) will stop execution and bring up a message box with the caption and data as it exists at that point in the program. Press "OK" to continue. You will be able to see what is going on. Just comment it out when you figure out what is wrong and get it running.

Is there a reason to dimension an array here? If all you are doing is copying a file and checking it line by line, you don't need to dimension an array. Just do it line by line: input a line from the source file, check it and decide whether to print it and discard or print it to the output file; then get the next line from the source file; continue until end of the source file (use a do/while loop as has been suggested). It is much simpler and a more general way to do it. Just a suggestion.

AM

Could you write in code what you mean?

 

[Ibix]

I've only used VBA, but a quick Google suggests that the simple debugging method I use in that ought to work here too.

VB.Net appears to have a "locals" window that will let you see the values of variables. Turn it on: https://msdn.microsoft.com/en-us/library/aa290840(v=vs.71).aspx. Set a simple breakpoint at the beginning of your function (https://msdn.microsoft.com/library/k80ex6de(v=vs.100).aspx). Run your code in debug mode (press F5, apparently). The program pauses when it gets to the instruction where you set the breakpoint. Then you can execute one statement at a time - press F11 to execute one statement (I think: https://msdn.microsoft.com/en-us/library/y740d9d3.aspx. It's F8 in VBA...). Look at the locals window to see what's happening to the variables as you go.

As I say, I've only used that in Excel VBA and I've never used VB.net. Might be worth investigating however - you'll only waste a couple of minutes if it doesn't work, and it's quite powerful if it does work.

 

[phion]

I've only used VBA, but a quick Google suggests that the simple debugging method I use in that ought to work here too.

VB.Net appears to have a "locals" window that will let you see the values of variables. Turn it on: https://msdn.microsoft.com/en-us/library/aa290840(v=vs.71).aspx. Set a simple breakpoint at the beginning of your function (https://msdn.microsoft.com/library/k80ex6de(v=vs.100).aspx). Run your code in debug mode (press F5, apparently). The program pauses when it gets to the instruction where you set the breakpoint. Then you can execute one statement at a time - press F11 to execute one statement (I think: https://msdn.microsoft.com/en-us/library/y740d9d3.aspx. It's F8 in VBA...). Look at the locals window to see what's happening to the variables as you go.

As I say, I've only used that in Excel VBA and I've never used VB.net. Might be worth investigating however - you'll only waste a couple of minutes if it doesn't work, and it's quite powerful if it does work.

I mean, it's pretty clear that the issue resides in the loops I'm using.

Is there a reason to dimension an array here? If all you are doing is copying a file and checking it line by line, you don't need to dimension an array. Just do it line by line: input a line from the source file, check it and decide whether to print it and discard or print it to the output file; then get the next line from the source file; continue until end of the source file (use a do/while loop as has been suggested). It is much simpler and a more general way to do it. Just a suggestion.

AM

When I remove the arrays and the for loop and do as you say, the same exact behavior results. Regardless of the name I type to be removed, the last name in the list is removed instead.

Console.WriteLine("Which player would you like to remove?")

        player = Console.ReadLine()

        While Not inFile.EndOfStream

            copyPlayer = inFile.ReadLine()

            If player <> copyPlayer Then

                outFile.WriteLine(copyPlayer)

            End If

        End While