24
\$\begingroup\$
for (var i = 1; i <= 100; ++i) {
    var fizzBuzz = ""

    if i % 3 == 0 {
        fizzBuzz += "Fizz"
    }

    if i % 5 == 0 {
        fizzBuzz += "Buzz"
    }

    if fizzBuzz == "" {
        fizzBuzz += "\(i)"
    }

    println(fizzBuzz)
}

I don't really like comparing strings with ==, but apparently, that's how you do it in Swift (and there's not another option).

The parenthesis in if statements are optional in Swift. Should that be a thing, or should we stick with them? The curly-braces were optional in Objective-C (and lots of programming languages) but they're not in Swift. Despite their former optionality, I never thought it was a good idea to not use them--is it a good idea to not use parenthesis here?

Not explicitly declaring the type of variable is now a thing in Swift (although the variable still has an explicit type, it's just implicitly determined). Is it okay to let the type be implicitly determined, or should we stick with explicitly declaring the type?

\$\endgroup\$
46
\$\begingroup\$

Is this FizzBuzz Swift-y?

Kinda, but it could be a lot better. Here's what I would do to fix it:

  • Extrapolate this code into a method, then call the method from the for loop.

    func fizzbuzz(i: Int) -> String
    {
       // ...
    }
    
  • There is a handy Swift feature called "Tuples". Tuples are groupings of values. We can use them to represent our results from the modulo operation.

    let result = (i % 3, i % 5)
    
  • Now that we are using a Tuple to represent the result, we can easily use a switch to perform the necessary actions.

    switch result
    {
        case (0, 0):
            return "FizzBuzz"
        case (0, _):
            return "Fizz"
        case (_, 0):
            return "Buzz"
        default:
            return "\(i)"
    }
    

    You might be wondering what that underscore is. That _ is used to indicate a discarded value that we don't really care about, the value in the tuple that doesn't really matter to us in the evaluation.

  • Your for loop isn't very "Swift-y". Here's how I would write is so that it calls the function 100 times.

    for number in 1...100
    {
        println(fizzbuzz(number))
    }
    

Answers to your questions

  • The parenthesis in if statements are optional in Swift. Should that be a thing, or should we stick with them?

    This is a very style-oriented question. Style differs from person to person, and is very organic. Since no "style rules" have been put in place, do whatever you are most comfortable with.

  • The curly-braces were optional in Objective-C (and lots of programming languages) but they're not in Swift.

    This was implemented in order to prevent simplified if conditional statements and the class of bugs that associated with them. For example:

    if (someCondition)
        doThisForSomeCondition()
        doThisAsWell()
    if (someOtherCondition)
    // ...
    

    Swift's forced usage of braces eliminated the execution of doThisAsWell() outside of the someCondition conditional statement.

  • Is it okay to let the type be implicitly determined, or should we stick with explicitly declaring the type?

    Whether or not you include the explicit type is a matter of taste. In some contexts it might make your code more readable. However, this is usually not the case; and since the compiler will almost never assign the wrong type to your variable, I usually leave the type to be implicitly determined. Whatever way you decide will not affect speed/efficiency of the code, so that is not a factor.

\$\endgroup\$
  • \$\begingroup\$ +1 for the for number in 1...100 suggestion. I don't think the tuple and switch implementation is an improvement over the original. Tuples are useful sometimes, but I don't think this is one of those times. I see code duplication; "Fizz" and "Buzz" appear twice now. Also, the tuple solution gets clumsy if you try to extend it to "FizzBuzzPling" as suggested here: codereview.stackexchange.com/questions/56708/… \$\endgroup\$ – GraniteRobert Jul 12 '14 at 2:24
  • 1
    \$\begingroup\$ @GraniteRobert This is due to the design of the language: "In contrast with switch statements in C and Objective-C, switch statements in Swift do not fall through the bottom of each case and into the next one by default. Instead, the entire switch statement finishes its execution as soon as the first matching switch case is completed, without requiring an explicit break statement." If the design were more C-like, it would be a lot less awkward to extend it. \$\endgroup\$ – syb0rg Jul 12 '14 at 2:36
  • \$\begingroup\$ @syb0rg: Yes, I understand (and completely approve of) the fact that Swift cases don't fall through like they do in C. But with either style of switch, I think it's an awkward solution to the current problem. Maybe I just need more time to get used to switch statements where the cases can overlap and the order of cases matters. \$\endgroup\$ – GraniteRobert Jul 12 '14 at 2:47
  • \$\begingroup\$ Your cited guide doesn't match with common Ruby best practices. Ruby Style Guide is clearer: • Use def with parentheses when there are arguments. Omit the parentheses when the method doesn't accept any arguments. • Don't use parentheses around the condition of an if/unless/while/until. • Omit parentheses around parameters for methods that are part of an internal DSL, methods that have "keyword" status in Ruby (e.g. attr_reader, puts) and attribute access methods. Use parentheses around the arguments of all other method invocations. \$\endgroup\$ – 200_success Jul 12 '14 at 2:55
  • 7
    \$\begingroup\$ We're not talking about Ruby--we're talking about Swift. :/ \$\endgroup\$ – nhgrif Jul 12 '14 at 12:06
12
\$\begingroup\$

In a for loop in Swift, the parentheses are optional, and from all of Apple's book and sample code, they are usually omitted--you did so yourself on the if statements. Furthermore, Swift has a range operator (two, in fact), so you should use that instead of the manual increments anyway.

for i in 1...100 {
    ...
}

For better or worse, == 0 is the only way to do the comparison in Swift, and many would argue it's easier to read in C/ObjC anyway.

\$\endgroup\$
7
\$\begingroup\$

A couple of observations around these lines…

if fizzBuzz == "" {
    fizzBuzz += "\(i)"
}
  • This seems like an unjustified use of string interpolation. String(i) would be more direct.
  • Why bother concatenating to an empty string?

With those two changes…

if fizzBuzz == "" {
    fizzBuzz = String(i)
}
\$\endgroup\$
6
\$\begingroup\$

Optional Parentheses

I haven't seen any good reasons to use parentheses where Swift says they are optional. It makes no more sense to use them in an if or for than it does in a simple math expression like foo = (x + y). Or the C incantation that some folks believe has mysterious powers: return (foo);

If you want your Swift to look more like C, go ahead and add the optional parentheses. But that seems an unworthy goal to me, and doomed to failure. :-)

Comparison with ==

I have the opposite view; strings are ordinary objects in Swift, the ordinary comparison operators like == seem reasonable to me. Building Objective-C on top of C forced some weird practices on us. I'm willing to let them go.

\$\endgroup\$
5
\$\begingroup\$

Here is a simple FizzBuzz I hacked up in the playground:

var multiple:Bool

for number in 1...100 {
    multiple = false
    if number % 3 == 0 {
        print("Fizz")
        multiple = true
    }
    if number % 5 == 0 {
        print("Buzz")
        multiple = true
    }
    if !multiple {
        print(number)
    }
    print("\n")
}

The tuple examples are nice and demonstrate a language feature, but I think the following is just as efficient and easy to read. The boolean flag should be as efficient or potentially more efficient than the empty string checks.

This assumes you just want to dump to console. Building a string and returning it would be an easy modification to make as well.

\$\endgroup\$
4
\$\begingroup\$

I don't know if this is Swift-y or Ug-ly, or maybe both.

I tried to make something similar to OldCurmudgeon's data-centric FizzBuzzPling extension in Java I didn't get far with enum, but found a way to implement it with struct:

struct FizzBuzz
{
    var s: String
    var n: Int

    init(_ s: String, _ n: Int)
    {
        self.s = s
        self.n = n
    }

    func fizz(number: Int) -> String
    {
        return (number % self.n) == 0 ? self.s : ""
    }
}

let fizzBuzzArray = [FizzBuzz("Fizz", 3),
                     FizzBuzz("Buzz", 5),
                     FizzBuzz("Pling", 7)]

func fizzbuzz(number: Int) -> String
{
    var result = ""
    for fb in fizzBuzzArray
    {
        result += fb.fizz(number)
    }
    return (result == "") ? String(number) : result
}

for number in 1...106
{
    println(fizzbuzz(number))
}

The initializer for FizzBuzzArray seems too verbose, but I don't know a way to simplify it.

I borrowed the String(number) suggestion from 200_success.

\$\endgroup\$
2
\$\begingroup\$

I like keeping Swift tight, personally, using in-line operators. Here's my solution:

for i in 1...100 {
    print( i % 3 != 0 && i % 5 != 0 ? i : (i % 3 == 0 ? "fizz" : "")+(i % 5 == 0 ? "buzz" : ""))
}
\$\endgroup\$
  • 1
    \$\begingroup\$ O my God. is it violated clean code? \$\endgroup\$ – Nazmul Hasan Mar 15 '17 at 15:40
2
\$\begingroup\$

I would encapsulate the code in a function:

func fizzBuzz(i: Int) -> String {
    var str = "" //Allows concatenation, eliminating the need for redundant "Fizz" and "Buzz" Strings
    if i % 3 == 0 {
        str += "Fizz"
    }
    if i % 5 == 0 {
        str += "Buzz"
    }
    //If evenly divisible by both 3 and 5, str = FizzBuzz
    if str == "" {
        str = String(i)
    }
    //Else str = i
    return str
}

But instead of using a loop, I would use:

(1 ... 100).forEach { print(fizzBuzz($0)) }

Here, we create an instance of the Sequence protocol (a CountableRange) containing the numbers 1 to 100. By conforming to the Sequence protocol, we get access to the forEach function, which takes a closure as its argument.

In Swift, closures are discrete blocks of code that can be called from elsewhere in the program (functions are just special cases of closures). When a function expects a closure, and can guarantee what the closure's argument type is, the argument can be referred to as $0. Moreover, when using single line closures, the function's parenthesis can be omitted.

The above code runs the specified closure (our function) for each element of the range, and prints the result.

\$\endgroup\$
1
\$\begingroup\$

How about Collections, closures and map function?

let output : [String] = Array<Int>(1...100).map { num in
    var str : String = ""
    if num % 3 == 0 {
        str += "Fizz"
    }
    if num % 5 == 0 {
        str += "Buzz"
    }
    return str.isEmpty ? String(num) : str
}

print(output.joined(separator:"\n"))

This solution starts with an array of 100 integers (1...100), then that array is mapped to an array of same number of strings with a transformation function in a closure. The closure evaluates each of the stored integers, finds out modulo of 3 and 5 and builds the string for the mapped element.

Advantages over a for loop are in the flexibility of the execution. Since each array element maps to exactly one array element in the output string array the execution order is not relevant and can be optimised and paralleled by the implementation of the map function. As this is an assignment - the entire program is one assignment (let output), executions of this kind can be deferred to the point of usage such as lazy var or asynchronous with Promise or Future.

Since the output is stored in the array, we can use Collection functions to further work on the array - filtering, sorting and more - in the example shown function joined that produces the entire output string in one go.

\$\endgroup\$
  • 1
    \$\begingroup\$ You have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and why it is better than the original) so that the author and other readers can learn from your thought process. \$\endgroup\$ – Dannnno Mar 15 '18 at 15:27

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.