Take input of delimited file of integers and output result

Question

I wanted to learn some new language so I developed a really simple vb.net "console application" (was that the right type?) that takes text input, looks at it and multiplies it if it's even and then reads each result into a msgbox.

• test.txt = 1 2 3 4 5 6 7 8 9
• result should be an array {1 4 3 8 5 12 7 16 9}

This would be my first vb.net programming attempt

Module MyFirstModule
    Sub Main()
        HelloNumbers()
    End Sub

    Private Sub HelloNumbers()
        Dim inputFile As String
        inputFile = "c:\temp\test.txt"
        Dim i As Integer
        Dim readResult As String() = Split(System.IO.File.ReadAllText(inputFile))
        Dim multipliedResult As Integer()
        Dim lower As Integer = LBound(readResult)
        Dim upper As Integer = UBound(readResult)
        ReDim multipliedResult(0 To upper)
        For i = lower To upper
            multipliedResult(i) = MultiplyEvenNumbers(CInt(readResult(i)))
        Next

        For i = lower To upper
            If multipliedResult(i) Mod 2 = 0 Then
                MsgBox(readResult(i) & " was multiplied by two for " & multipliedResult(i))
            Else : MsgBox(readResult(i) & " is original")
            End If
        Next
    End Sub

    Private Function MultiplyEvenNumbers(ByVal inputNumber As Integer) As Integer
        If inputNumber Mod 2 = 0 Then inputNumber = inputNumber * 2
        MultiplyEvenNumbers = inputNumber
    End Function
End Module

Answer 1

This is a good start. You created two functions to separate parts of the logic but there is more that can be refactored. The HalloNumbers sub is too big and does too much. Let's try to improve.

First the path shouldn't be hardcoded inside it so we pass it as parameter:

Sub Main()
    HelloNumbers("c:\temp\test.txt")
End Sub

Now we take parts of the big HalloNumbers and move them into fuctions that do only one thing - we always strive for this because it's easier to maintain when you only have to care about one thing at a time (see SRP).

Private Sub HelloNumbers(ByVal inputFile As String)
    Dim numbers As Integer() = ReadNumbers(inputFile)
    Dim multipliedNumbers = numbers.Select(Function(number) MultiplyEvenNumbers(number))
    PrintNumbers(multipliedNumbers)
End Sub

So we can extract the following functions:

We need to read the numbers. They are in a file. So let's require a path and after reading them we convert each number-string into an Integer with linq:

Private Function ReadNumbers(ByVal inputFile As String) As IEnumerable(Of Integer)
    ReadNumbers = Split(File.ReadAllText(inputFile)).Select(Function(s, i) CInt(s))
End Function

Then we need to multiply them. This one was already there so we leave it as is.

Private Function MultiplyEvenNumbers(ByVal inputNumber As Integer) As Integer
    If inputNumber Mod 2 = 0 Then inputNumber = inputNumber * 2
    MultiplyEvenNumbers = inputNumber
End Function

Finaly you want to print the results. Let's do it in PrintNumbers. If you decide to write the output to the console you now just have to adjust this small function.

Private Sub PrintNumbers(ByVal numbers As IEnumerable(Of Integer))
    For Each number As Integer In numbers
        If number Mod 2 = 0 Then
            MsgBox(String.Format("{0} was multiplied by two for {1}", number, number / 2))
        Else : MsgBox(String.Format("{0} is original", number))
        End If
    Next
End Sub

Answer 2

You undoubtedly have this line at the top of your code file:

Imports Microsoft.VisualBasic

Remove it, and your code stops compiling. Why? Because you're using VB6-like constructs that this namespace provides compatibility for.

More precisely, the MsgBox function. Use the MessageBox class in the Microsoft.Windows.Forms namespace instead, for a much more .NET-idiomatic invocation:

MessageBox.Show(String.Format("..."))

LBound and UBound will also stop compiling without the Microsoft.VisualBasic namespace imported.

Arrays in VB.NET are always 0-based, and an array's length can be determined using its... Length property: that's just one of the awesome implications of .NET's everything is an object.

Therefore, instead of this:

Dim lower As Integer = LBound(readResult)
Dim upper As Integer = UBound(readResult)
For i = lower To upper

You can do that:

For i = 0 To readResult.Length - 1

Also, instead of using ReDim to resize an array:

Dim multipliedResult As Integer()
ReDim multipliedResult(0 To upper)

You could use the more idiomatic Array.Resize(ref multipliedResult, readResult.Length)... but then, why resize the array when you could just declare it with the appropriate length in the first place? Or better, why even use an array, when you could use a List(Of String) and just .Add values and let the framework deal with the underlying array? =)

Now, the array itself is obtained using the Split function, also a courtesy of Microsoft.VisualBasic - the .NET-idiomatic way to split a string is to use the Split method, available on any instance of any String object - because yes, even a String is an object in .NET!

Dim a As String = "foo bar baz"
Dim b As String() = a.Split(new String(){Environment.NewLine}, StringSplitOptions.None)

But you don't even need to Split anything, because you can ReadAllLines instead of ReadAllText, and work with a string array directly - add Imports System.IO at the top of the module, and reduce the wordiness of System.IO.File, too:

Dim readResult As String() = File.ReadAllLines(inputFile)

Another thing that makes your VB.NET feel like VBA/VB6 code, is how you're returning from a function:

If inputNumber Mod 2 = 0 Then inputNumber = inputNumber * 2
MultiplyEvenNumbers = inputNumber

In VB.NET, you use the Return keyword instead:

If inputNumber Mod 2 = 0 Then inputNumber = inputNumber * 2
Return inputNumber

But there's more: VB.NET has combined assignment operators, so instead of this:

inputNumber = inputNumber * 2

You can do this:

inputNumber *= 2

So to increment by one, you can't quite do i++ like you would in C#, but you can combine the + and = and do i += 1 to increment, and i -= 1 to decrement.

The CInt conversion function is also VB6-ish; in .NET you convert things, so instead of this:

CInt(readResult(i))

You would have that:

Convert.ToInt32(readResult(i))

Although, that's just one of many ways to convert types in VB.NET.

Lastly, idiomatic .net code will always have Option Explicit and Option Strict turned on... and you can reduce the verbosity of declarations by enabling Option Infer, so the compiler will automatically infer the correct type, so instead of this:

Dim foo As String = "something"

You can have that:

Dim foo = "something"

And foo is known at compile time (read: it's not a dynamic/runtime thing) to be a String.

---

What about LINQ?

As mentioned above, an array is an object. But it's not just any object - it implements an interface called IEnumerable, which means the LINQ extension methods can be used to iterate it, instead of an explicit For loop:

Dim readResult = File.ReadAllLines(inputFile)
Dim multipliedResults = From line In readResults
                        Select MultiplyEvenNumbers(Convert.ToInt32(line))

But you'll probably want to learn a bit about closures and delegates before diving into this.