9
\$\begingroup\$

Write a program that prints the numbers from 1 to 100. But for multiples of three print “Fizz” instead of the number and for the multiples of five print “Buzz”. For numbers which are multiples of both three and five print “FizzBuzz”

It seems that we're shooting for one question for each language. I'm sick of FizzBuzz, so I'm killing two birds with one stone. This code works in both and .

My goal here was to write a flexible Fizzbuzzer; one that could double as a Hello World program (of sorts). Is there anything left to say about it, or are we all FizzBuzzed out?

FizzBuzzer (Standard Module):

Option Explicit

Public Function Convert(ByVal number As Integer, Optional ByVal fizzDivisor As Integer = 3, Optional ByVal buzzDivisor As Integer = 5, Optional ByVal fizzWord As String = "Fizz", Optional ByVal buzzWord = "Buzz")

    If (number Mod fizzDivisor = 0) And (number Mod buzzDivisor = 0) Then
        Convert = fizzWord & buzzWord
    ElseIf (number Mod fizzDivisor = 0) Then
        Convert = fizzWord
    ElseIf (number Mod buzzDivisor = 0) Then
        Convert = buzzWord
    Else
        Convert = number
    End If

End Function

Example Calls:

Public Sub Main()

    Debug.Print "Standard Fizz Buzz"
    Dim i As Integer
    For i = 1 To 100
        Debug.Print (FizzBuzzer.Convert(i))
    Next i

    Debug.Print "Change the Numbers"
    For i = 1 To 100
        Debug.Print (FizzBuzzer.Convert(i, 4, 6))
    Next i

    Debug.Print "Change the Words"
    For i = 1 To 100
        Debug.Print (FizzBuzzer.Convert(i, fizzWord:="Hello", buzzWord:="World"))
    Next i

End Sub
\$\endgroup\$
  • 3
    \$\begingroup\$ ++ for the question. Mine is probably an overkill but you may find THIS interesting \$\endgroup\$ – user28366 Aug 6 '14 at 9:28
4
\$\begingroup\$

Just a slight optimization: instead of evaluating the modulo twice just declare two booleans an evaluate them upfront.

Public Function Convert(ByVal number As Integer, _
                        Optional ByVal fizzDivisor As Integer = 3, _
                        Optional ByVal buzzDivisor As Integer = 5, _
                        Optional ByVal fizzWord As String = "Fizz", _
                        Optional ByVal buzzWord = "Buzz")

    Dim fizzState As Boolean
    fizzState = (number Mod fizzDivisor = 0)

    Dim buzzState As Boolean
    buzzState = (number Mod buzzDivisor = 0)

    If fizzState And buzzState Then
        Convert = fizzWord & buzzWord
    ElseIf fizzState Then
        Convert = fizzWord
    ElseIf buzzState Then
        Convert = buzzWord
    Else
        Convert = number
    End If

End Function

Also you may not want to include as many newlines in the arguments as I did but you don't want a line over 200 characters long or use a horizontal scroll-bar.

\$\endgroup\$
  • \$\begingroup\$ Your function lacks a return type \$\endgroup\$ – jmoreno Jan 13 at 23:35
2
\$\begingroup\$

FizzBuzzer

  • The Convert declaration fails to specify that it should return a String. As it is, the function returns a Variant. So sometimes it returns an integer, sometimes it returns a string. The declaration should be:

    Public Function Convert(...) As String
    
  • @ptwales is correct. Line continuations would be justified and welcomed in the function declaration.

  • Exit Function can be used to return early.

    If (number Mod fizzDivisor = 0) And (number Mod buzzDivisor = 0) Then
        Convert = fizzWord & buzzWord
        Exit Function
    ElseIf (number Mod fizzDivisor = 0) Then
        '...
    
  • value would be a better parameter name than number.

  • fizzWord and buzzWord are already being concatenated, so there's no need for the first check if we add a result variable. (Which incidentally removes the ability to return early.)

    Public Function Convert(ByVal value As Integer, _
                    Optional ByVal fizzDivisor As Integer = 3, _
                    Optional ByVal buzzDivisor As Integer = 5, _
                    Optional ByVal fizzWord As String = "Fizz", _
                    Optional ByVal buzzWord = "Buzz") As String
    
        Dim result As String 
    
        If (value Mod fizzDivisor = 0) Then
            result = fizzWord
        End If
    
        If (value Mod buzzDivisor = 0) Then
            result = result & buzzWord
        End If
    
        If result = vbNullString Then
            result = value
        End If
    
        Convert = result
    
    End Function
    

Main

  • There shouldn't be logic in the Main routine, only calls to other subs/functions. Each of the loops should be broken into their own subroutines.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.