🔆 Bright side

Your code gives the correct output and delivers on the premise of extensibility.

📝 Suggestions

Here are some suggestions:

1. The namekey is a name that doesn't tell a lot about the nature of the tuple. If you feel that it is appropriate, that would conflict withresponsesByMultiples. Then It would have made more sense for the latter to be namedkeys. Personally, I'd prefer to callresponses orrules.

2. Theserules expressed by a tuple without labels is a little bit confusing. Better use a struct, or just add labels.

3. You can easily decompose a tuple this way:

   for (multiple, response) in responsesByMultiples {    ...}

4. Calling the first elementmultiple is a little bit too optimistic (or pessimistic depending on the way you to see things), it presumes that there is a high probability thatn is actually a multiple ofmultiple. It would make more sense to me to name the first element of the tuplesdividsor.

5. n is a bit too generic, usedividend instead.

6. You don't have to specify the type inisMultiple: Bool, type inference can do the job for you. Generally speaking, specifying the type can help in reducing the compile times, but in this case, it wouldn't make a difference.

7. Instead of using(n % multiple) == 0, there is a nicesyntax in Swift 5 :

   n.isMultiple(of: multiple)

   (this syntax exacerbates the problem mentioned in 4.)

8. To check that a String is empty, it is more efficient to check the.isEmpty property.Here is a benchmark that confirms it:

   import Foundationlet array = (0..<1_000_000).map { _ in    Double.random(in: 0..<1) < 0.8 ? "" : String("Hello".shuffled())}do {    let start = Date()    let result = array.filter { $0 == "" }    let end = Date()    print(result.count, end.timeIntervalSince(start))}do {    let start = Date()    let result = array.filter { $0.isEmpty }    let end = Date()    print(result.count, end.timeIntervalSince(start))}

   The execution times are respectively44ms and34ms.

💔 Putting it all together ❤️

Here a version of your code that takes the previous points into account :

func fizzBuzz (    number dividend: Int,    rules: [(Int, String)] = [(3, "Fizz"), (5, "Buzz")]    ) -> String {    var result: String = ""    for (divider, response) in rules        where dividend.isMultiple(of: divider) {        result += response    }    return result.isEmpty ? String(dividend) : result}

🧐 Further reading

You can find many implementations of this classic question on Code Review.Here is a quite informative one.

• 1
  \$\begingroup\$Damn that is a very well formatted answer! I agree with every point on the list, except maybe #6 because I personally like using explicit typing. Thanks!\$\endgroup\$

  Tiago Marinho

  – Tiago Marinho

  2019-08-03 00:38:57 +00:00

  CommentedAug 3, 2019 at 0:38

• swift
• fizzbuzz