Improving efficiency

[theazza]

is there a more efficient way of coding this

Dim lesson As String = cmbSession.SelectedIndex
        Dim changed As String = lstNames.SelectedItem
        Dim chosen As String = cmbChange.SelectedIndex

        If lstupdate.Items.Count = 10 Then
            MsgBox("Classes can have a maximum of 10 students")
            Return
        End If

        lstNames.Items.Remove(changed)
        lstupdate.Items.Add(changed)

        If lesson = 0 Then
            Dim copyback1 As StreamWriter = File.CreateText("SaturdayCloud.txt")
            For Each entry1 As Object In lstNames.Items
                copyback1.WriteLine(entry1)
            Next
            copyback1.Close()

        ElseIf lesson = 1 Then
            Dim copyback2 As StreamWriter = File.CreateText("wednesdaycloud.txt")
            For Each entry2 As Object In lstNames.Items
                copyback2.WriteLine(entry2)
            Next
            copyback2.Close()

        ElseIf lesson = 2 Then
            Dim copyback3 As StreamWriter = File.CreateText("wednesdaymotoko.txt")
            For Each entry3 As Object In lstNames.Items
                copyback3.WriteLine(entry3)
            Next
            copyback3.Close()

        ElseIf lesson = 3 Then
            Dim copyback4 As StreamWriter = File.CreateText("tuesdayMotoko.txt")
            For Each entry4 As Object In lstNames.Items
                copyback4.WriteLine(entry4)
            Next
            copyback4.Close()
        End If


        If chosen = 0 Then
            Dim saveback1 As StreamWriter = File.CreateText("SaturdayCloud.txt")
            For Each entry1 As Object In lstupdate.Items
                saveback1.WriteLine(entry1)
            Next
            saveback1.Close()

        ElseIf chosen = 1 Then
            Dim saveback2 As StreamWriter = File.CreateText("wednesdaycloud.txt")
            For Each entry2 As Object In lstupdate.Items
                saveback2.WriteLine(entry2)
            Next
            saveback2.Close()

        ElseIf chosen = 2 Then
            Dim saveback3 As StreamWriter = File.CreateText("wednesdaymotoko.txt")
            For Each entry3 As Object In lstupdate.Items
                saveback3.WriteLine(entry3)
            Next
            saveback3.Close()

        ElseIf chosen = 3 Then
            Dim saveback4 As StreamWriter = File.CreateText("tuesdayMotoko.txt")
            For Each entry4 As Object In lstupdate.Items
                saveback4.WriteLine(entry4)
            Next
            saveback4.Close()
        End If

 

[JohnH]

You probably mean coding efficiency, which in this case means looking for code to refactor. Look for code that looks similar, perhaps you can move that code to a common place.

What I see here is that you have identical code in all your If blocks. The only thing that differs is that you have used different variable names, which has no relevance for the code that is executed, and that the filename string is different. So what you can do is to move one of those blocks to after the If statement, and within the If statement you just set different filename to a variable, use this variable as parameter to the CreateText function. Pseudo code:

dim name as string
if x then
    name = this
elseif y then
    name = that
end if

blah...CreateText(name)
blah blah

Another refactoring tip is if you need to call same code block from multiple different places, then you move that code block to a separate method, providing parameters for input data as needed.

 

[theazza]

where it streamwriter (name) the 'name' comes with a green underline saying that it was used before assigning a value

 

[theazza]

your even create text for that matter

 

[JohnH]

where it streamwriter (name) the 'name' comes with a green underline saying that it was used before assigning a value

You can initialize the variable when declaring it to avoid that warning, for example explicitly set it to Nothing or an empty string.

 

[JohnH]

your even create text for that matter

Yes, but that is no different for each if block, so that basically just adds duplicate code to each of them.

 

[theazza]

can you please give an example code of when you say blah, what exactly should go there, because an error message still appears - green underline

 

[Flippedbeyond]

Sample

coding efficiency, which in this case means looking for code to refactor. Look for code that looks similar, perhaps you can move that code to a common place.

John H - If i am understanding correctly, i think your basically just talking about shorten the code by removing some code redundancy right?

Here's an example, i chose to use a dictionary rather than if statements, you can also use a select case block if you'd like to. i just picked the dictionary in this case.

    Sub SomeMethodElsewhere()
        Dim lesson As Integer = cmbSession.SelectedIndex
        Dim changed As String = lstNames.SelectedItem
        Dim chosen As Integer = cmbChange.SelectedIndex

        '...
           'some of your other code..
        '...

        Dim dictNames As New Dictionary(Of Integer, String)

        dictNames.Add(0, "SaturdayCloud.txt")
        dictNames.Add(1, "wednesdaycloud.txt")
        '.... and so forth

        outputToFile(dictNames, lstNames, lesson)
        outputToFile(dictNames, lstupdate, chosen)
    End Sub

    Sub outputToFile(ByVal filelocation As Dictionary(Of Integer, String), ByRef lstFrom As ListBox, ByVal selectedIndex As Integer)
        Dim writer As StreamWriter = File.CreateText(filelocation(selectedIndex))
        For Each entry As Object In lstFrom.Items
            writer.WriteLine(entry)
        Next
        writer.Close()
    End Sub

    'alternative overloaded method
    Sub outputToFile(ByVal filelocation As String, ByRef lstFrom As ListBox)
        Dim writer As StreamWriter = File.CreateText(filelocation)
        For Each entry As Object In lstFrom.Items
            writer.WriteLine(entry)
        Next
        writer.Close()
    End Sub

 

[theazza]

what other methods can u suggest besides using dictionary

 

[Flippedbeyond]

Pseudo code:

dim name as string
if x then
    name = this
elseif y then
    name = that
end if

blah...CreateText(name)
blah blah

Another refactoring tip is if you need to call same code block from multiple different places, then you move that code block to a separate method, providing parameters for input data as needed.

just like John H said, just use a if statement and/or move the code block to a separate method

this example is referring to the alt. overloaded method i provided earlier

        Dim fileName As String = ""

        If lesson = 0 Then
            fileName = "SaturdayCloud.txt"
        ElseIf lesson = 1 Then
            fileName = "wednesdaycloud.txt"
        ElseIf lesson = 2 Then
            '...and so forth
        End If

        'or you can do this instead ' basically the same thing, a select could be more clear in this case
        ''you can also do 
        Select Case lesson
            Case Is = 0
                fileName = "SaturdayCloud.txt"
            Case Is = 1
                fileName = "wednesdaycloud.txt"
            Case Is = 2
                '...and so forth
        End Select

        'from the code i posted above
        outputToFile(fileName, lstNames)

        'or you can just write the code here... rather than using a method

 

[JohnH]

can you please give an example code of when you say blah, what exactly should go there, because an error message still appears - green underline

This is the code in one of your If blocks:

Dim copyback1 As StreamWriter = File.CreateText("SaturdayCloud.txt")
For Each entry1 As Object In lstNames.Items
copyback1.WriteLine(entry1)
Next
copyback1.Close()

As I explained all your If blocks have that same code, only the file name supplied to CreateText is different. So 'blah' is that part of your code.