patternMinor
FizzBuzz Kickoff
Viewed 0 times
kickofffizzbuzzstackoverflow
Problem
My first FizzBuzz question (well, I asked, then deleted, one previously), and my very first VB.NET question.
Please tell me the good, bad, and awful so I do, don't, and don't do the same in my next venture into VB land.
Module Module1
Function FizzBuzzify(ByVal Value As Integer) As String
If Value Mod 5 = 0 And Value Mod 3 = 0 Then
Return "FizzBuzz"
End If
If Value Mod 3 = 0 Then
Return "Fizz"
End If
If Value Mod 5 = 0 Then
Return "Buzz"
End If
Return Value
End Function
Sub FizzBuzz(ByVal MinValue As Integer, ByVal MaxValue As Integer)
For index = MinValue To MaxValue
Console.WriteLine(FizzBuzzify(index))
Next
End Sub
Sub Main()
FizzBuzz(1, 100)
End Sub
End ModulePlease tell me the good, bad, and awful so I do, don't, and don't do the same in my next venture into VB land.
Solution
The Good
I don't see single-letter identifiers, you're passing parameters
The Bad
Procedures are implicitly
Hence, you should always use an explicit access modifier for all members in VB; the only method that I would expect to be
The Awful
Nothing awful here, really. Well done!
I don't see single-letter identifiers, you're passing parameters
ByVal explicitly, and you've extracted the logic into small, specialized procedures.The Bad
Procedures are implicitly
Public here; while consistent with the vb6 ancestor, it's confusing because the implicit access modifier for module/class members in other .net languages, is private.Hence, you should always use an explicit access modifier for all members in VB; the only method that I would expect to be
Public in that module, is Sub Main.The Awful
Nothing awful here, really. Well done!
Context
StackExchange Code Review Q#85824, answer score: 3
Revisions (0)
No revisions yet.