# Fizzbuzz of course

My second attempt with - I tried taking the advice from yesterday's question into consideration, but I did not try using LINQ.

You know the drill:

A programming challenge that prints the numbers from 1 to 100, where multiples of three print "Fizz," multiples of five print "Buzz," and multiples of both three and five print "FizzBuzz."

• Input is a list of integers 1-100 comma-separated in a txt file.
• Output is the list of fizzbuzz in the same format.

Code:

Option Explicit On
Option Strict On
Option Infer On
Option Compare Text
Imports System.Windows.Forms
Imports System.IO

Module FizzBuzz

Sub Main()
Dim arrayOfStrings As String() = GetInput("C:\temp\fizzbuzz.txt")
Dim arrayOfIntegers = ConvertMe(arrayOfStrings)
arrayOfStrings = FizzBuzzMe(arrayOfStrings, arrayOfIntegers)
Dim concatenatedArray As String = WillYouBeMine(arrayOfStrings)
WriteFizzBuzzFile(concatenatedArray)
End Sub

Private Function GetInput(ByVal path As String) As String()
Dim returnText As String = File.ReadAllText(path)
Dim returnArray As String() = returnText.Split(New String() {","}, StringSplitOptions.None)
Return returnArray
End Function

Private Function ConvertMe(ByVal arrayOfStrings As String()) As Integer()
Return Array.ConvertAll(arrayOfStrings, Function(str) Int32.Parse(str))
End Function

Private Function FizzBuzzMe(ByVal arrayOfStrings As String(), ByVal arrayOfIntegers As Integer()) As String()
Dim i As Integer
For i = 0 To arrayOfStrings.Length - 1
If Not arrayOfIntegers(i) Mod 15 = 0 Then
If arrayOfIntegers(i) Mod 3 = 0 Then
arrayOfStrings(i) = "fizz"
ElseIf arrayOfIntegers(i) Mod 5 = 0 Then
arrayOfStrings(i) = "buzz"
End If
Else : arrayOfStrings(i) = "fizzbuzz"
End If
Next
Return arrayOfStrings
End Function

Private Function WillYouBeMine(ByVal stringArray As String()) As String
Return String.Join(",", stringArray)
End Function

Private Sub WriteFizzBuzzFile(ByVal concatenatedArray As String)
Const NEW_FILE As String = "C:\Temp\fizzbuzzreturn.txt"
Dim fileAuthor As New StreamWriter(NEW_FILE)
If Not File.Exists(NEW_FILE) Then File.CreateText(NEW_FILE)
fileAuthor.WriteLine(concatenatedArray)
fileAuthor.Close()
End Sub

End Module

• Why the -1 vote? Is this a bad question? I know it's played out, but this would be a fine question in VBA, so please let me know what it's missing so I can fix it. – Raystafarian Sep 16 '16 at 18:03
• I guess fizzbuzz just isn't popular. -1 for lack of imagination! – Raystafarian Sep 29 '16 at 21:23

## 4 Answers

You said , which is good. I'm going to go into a few basic VB.NET constructs that should help you, and then cover a couple advanced ones in detail so that hopefully it's useful for future projects. :)

I know you said you didn't try to use LINQ, but I added an example of it (you can ignore it if you like) that shows the other form of LINQ that wasn't demonstrated in your last question. This may make it easier to understand.

So, first: there's no need to define arrayOfStrings in the FizzBuzzMe method as a parameter. It's not used as such, instead it's used as a pre-fill so that you already have an array of the right size, but there are better ways to do that:

Private Function FizzBuzzMe(ByVal arrayOfIntegers As Integer()) As String()
Dim arrayOfStrings As New String(arrayOfIntegers.Length)
Dim i As Integer
For i = 0 To arrayOfStrings.Length - 1
If Not arrayOfIntegers(i) Mod 15 = 0 Then
If arrayOfIntegers(i) Mod 3 = 0 Then
arrayOfStrings(i) = "fizz"
ElseIf arrayOfIntegers(i) Mod 5 = 0 Then
arrayOfStrings(i) = "buzz"
End If
Else : arrayOfStrings(i) = "fizzbuzz"
End If
Next
Return arrayOfStrings
End Function


You should be Using the StreamWriter instead of opening and closing it manually. This is because StreamWriter implements IDisposable, which is almost always an indicator that it uses unmanaged resources, and should be properly freed. (In this case, .Close() will do this, but it's still a best practice to use the Using.)

You also do not need the File work in this method. It's automatically created when StreamWriter is opened.

Private Sub WriteFizzBuzzFile(ByVal concatenatedArray As String)
Const NEW_FILE As String = "C:\Temp\fizzbuzzreturn.txt"
Using fileAuthor As New StreamWriter(NEW_FILE)
fileAuthor.WriteLine(concatenatedArray)
End Using
End Sub


Generally, we recommend to use the alias type for all work instead of the strong type. The only time this occurs is in ConvertMe when you use Int32.Parse, usually we would use Integer.Parse.

Private Function ConvertMe(ByVal arrayOfStrings As String()) As Integer()
Return Array.ConvertAll(arrayOfStrings, Function(str) Integer.Parse(str))
End Function


The WillYouBeMine function has two problems: meaningless name, and it doesn't encapsulate a useful feature. It's just a slightly quicker way of combining the strings, but it has no actual impact except using stack space. Just get rid of it and inline the String.Join call.

Sub Main()
Dim arrayOfStrings As String() = GetInput("C:\temp\fizzbuzz.txt")
Dim arrayOfIntegers = ConvertMe(arrayOfStrings)
arrayOfStrings = FizzBuzzMe(arrayOfIntegers)
Dim concatenatedArray As String = String.Join(",", arrayOfStrings)
WriteFizzBuzzFile(concatenatedArray)
End Sub


Lastly, we're going to modify the algorithm in FizzBuzzMe just a bit. Instead of making three cases, we only need two.

Private Function FizzBuzzMe(ByVal arrayOfIntegers As Integer()) As String()
Dim arrayOfStrings(arrayOfIntegers.Length) As String
Dim i As Integer
For i = 0 To arrayOfStrings.Length - 1
arrayOfStrings(i) = ""

If i Mod 3 = 0 Then
arrayOfStrings(i) &= "fizz"
End If

If i Mod 5 = 0 Then
arrayOfStrings(i) &= "buzz"
End If
Next
Return arrayOfStrings
End Function


Because we initialize a new string array, it's guaranteed by .NET to be all null (Nothing) strings. This eliminates the need for the three If statements, and brings you down to just two conditions. Then you append fizz if it's a fizz line, and append buzz if it's a buzz line.

There is no need for GetInput to be as many lines as it is, we can shorten it into one. (ALL the variables are used in one location.)

Private Function GetInput(ByVal path As String) As String()
Return File.ReadAllText(path).Split(New String() {","}, StringSplitOptions.None)
End Function


We don't generally declare an iterator variable (i) outside the loop.

For i As Integer = 0 To arrayOfStrings.Length - 1


Now, you don't take any input in this programme at all. You just run it procedurally and exit. While this is great for dummy programme's that demonstrate a system, you should consider adding support for user input through arguments or the CLI. I'm not going to detail all that here, but I will say that taking input through the CLI is extremely easy.

The next thing we can do is replace the result of FizzBuzzMe with an IEnumerable(Of String), and change it to an Iterator function. This allows us to be lazy in our implementation. We simply Yield whatever value we have at the moment.

Private Iterator Function FizzBuzzMe(ByVal arrayOfIntegers As Integer()) As IEnumerable(Of String)
For i As Integer = 0 To arrayOfIntegers.Length - 1
Dim line As String = ""

If i Mod 3 = 0 Then
line &= "fizz"
End If

If i Mod 5 = 0 Then
line &= "buzz"
End If

Yield line
Next
End Function


If you notice, we got rid of arrayOfStrings entirely. Instead, we're using an Iterator Function to only return lines as we process them. This means that if we're only looking for the first fizz, we aren't processing the entire collection, but instead only process until we find fizz. (I'm not going to write an example of this at this time of night, but it's pretty cool.)

The only other change we need, is add .ToArray() to the FizzBuzzMe(arrayOfIntegers) call:

Sub Main()
Dim arrayOfStrings As String() = GetInput("C:\temp\fizzbuzz.txt")
Dim arrayOfIntegers = ConvertMe(arrayOfStrings)
arrayOfStrings = FizzBuzzMe(arrayOfIntegers).ToArray()
Dim concatenatedArray As String = String.Join(",", arrayOfStrings)
WriteFizzBuzzFile(concatenatedArray)
End Sub


Next, we'll do a little LINQ to make it pop just a bit:

Private Function FizzBuzzMe(ByVal arrayOfIntegers As Integer()) As IEnumerable(Of String)
Return arrayOfIntegers.Select(Function(x)
Dim value As String = ""
If x Mod 3 = 0 Then value &= "fizz"
If x Mod 5 = 0 Then value &= "buzz"
Return value
End Function)
End Function


Basically, the .Select method on an IEnumerable (which an array is) allows us to transform the elements from Integer to a String with some complex logic.

We can't (and don't need) to extend this to our Iterator version, as Select is already lazy.

Lastly, we can use the If(Expression, IfTrue, IfFalse) construct to simplify our FizzBuzzMe method:

Private Function FizzBuzzMe(ByVal arrayOfIntegers As Integer()) As IEnumerable(Of String)
Return arrayOfIntegers.Select(Function(x)
Return If(x Mod 3 = 0, "fizz", "") & If(x Mod 5 = 0, "buzz", "")
End Function)
End Function


You could even one-line it pretty easily at this point:

Private Function FizzBuzzMe(ByVal arrayOfIntegers As Integer()) As IEnumerable(Of String)
Return arrayOfIntegers.Select(Function(x) If(x Mod 3 = 0, "fizz", "") & If(x Mod 5 = 0, "buzz", ""))
End Function


Finally, the end of our code looks like:

Sub Main()
Dim arrayOfStrings As String() = GetInput("C:\temp\fizzbuzz.txt")
Dim arrayOfIntegers = ConvertMe(arrayOfStrings)
arrayOfStrings = FizzBuzzMe(arrayOfIntegers).ToArray()
Dim concatenatedArray As String = String.Join(",", arrayOfStrings)
WriteFizzBuzzFile(concatenatedArray)
End Sub

Private Function GetInput(ByVal path As String) As String()
Return File.ReadAllText(path).Split(New String() {","}, StringSplitOptions.None)
End Function

Private Function ConvertMe(ByVal arrayOfStrings As String()) As Integer()
Return Array.ConvertAll(arrayOfStrings, Function(str) Integer.Parse(str))
End Function

Private Function FizzBuzzMe(ByVal arrayOfIntegers As Integer()) As IEnumerable(Of String)
Return arrayOfIntegers.Select(Function(x)
Return If(x Mod 3 = 0, "fizz", "") & If(x Mod 5 = 0, "buzz", "")
End Function)
End Function

Private Sub WriteFizzBuzzFile(ByVal concatenatedArray As String)
Const NEW_FILE As String = "C:\Temp\fizzbuzzreturn.txt"
Using fileAuthor As New StreamWriter(NEW_FILE)
fileAuthor.WriteLine(concatenatedArray)
End Using
End Sub


We've reduced the complexity of certain portions of the code, and demonstrated a few complex code examples at the same time.

Input:

1,2,3,4,5,6,7,8,9,10,11,12,13,14,15


Output:

,,fizz,,buzz,fizz,,,fizz,buzz,,fizz,,,fizzbuzz


As you can see, the programme works just fine. I do hope you (and others) find this information helpful, and can learn more about VB.NET from it. :) Personally, I find VB.NET to be quite powerful, even though a lot of people act like it's not.

• Thank you, I've been indisposed for the last couple days, but I really appreciate the information, I'll definitely go over it soon. – Raystafarian Sep 29 '16 at 21:22

First, I would change the way you styled the if statements. Generally the less nesting you have the better.

I would go with the more simple:

If arrayOfIntegers(i) Mod 15 = 0 Then
arrayOfStrings(i) = "fizzbuzz"
ElseIf arrayOfIntegers(i) Mod 3 = 0 Then
arrayOfStrings(i) = "fizz"
ElseIf arrayOfIntegers(i) Mod 5 = 0 Then
arrayOfStrings(i) = "buzz"
End If


Second, some of the names of your functions are a bit confusing or don't provide as much information as they should. For example, ConvertMe, Convert to what? I would change it to something like ConvertToIntArray. What does WillYouBeMine do and what is its purpose?

Third, the method that reads input doesn't have the file name hard coded into it, instead it takes it as a parameter. I would do the same thing for the output.

Fourth, not entirely necessary, but I would consider renaming the integer and string arrays to input and output. Their current name tells us what sort of information they hold, but not really what the purpose of that information is.

• Thanks. Yeah I think I did a sub-optimal algorithm with my ifs, seeing yours. – Raystafarian Sep 16 '16 at 18:04

Just a couple of things to add to what @yitzih has already suggested:

Watch your imports

Try not to have imports in your code that you don't need. For example, in the code you've posted you've imported System.Windows.Forms even though you don't need it.

Split

Your split line seems overly verbose. The default behaviour is going to be SplitOptions.None, so you don't need to specify it. You can get away with just doing:

Dim returnArray As String() = returnText.Split({","c})


File Creation

You're performing a pointless check:

If Not File.Exists(NEW_FILE) Then File.CreateText(NEW_FILE)


This is done after Dim fileAuthor As New StreamWriter(NEW_FILE) which creates the file if it doesn't exist, so it's always going to be there for the File.Exists check. If you want to change the behaviour (from create new file to append, then you need to supply an extra parameter):

Dim fileAuthor As New StreamWriter(NEW_FILE, true)


Another way to go is to eliminate all the arrays and process each byte as it comes out of the stream. In many cases you'll find this faster than reading a string and converting it to a number:

Sub WriteResults(inFile As String, outFile As String)
Using inStream As New System.IO.StreamReader(inFile)
Using outStream As New System.IO.StreamWriter(outFile, False)
Do Until inStream.EndOfStream
Dim test As Integer = 0
'44 is ASCII code for the comma
Do Until inStream.Peek = 44 OrElse inStream.EndOfStream
test += inStream.Read - 48
Dim pek As Integer = inStream.Peek
If pek <> 44 AndAlso Not inStream.EndOfStream Then
test *= 10
test += inStream.Read - 48
pek = inStream.Peek
End If
Loop
inStream.Read()
If test Mod 3 = 0 Then
outStream.Write("fizz")
End If
If test Mod 5 = 0 Then
outStream.Write("buzz")
End If
If Not inStream.EndOfStream Then
outStream.Write(",")
End If
test = 0
Loop
End Using
End Using

End Sub