1

I am trying to implement the fizz buzz problem using maps in go lang. However, this code requires improvement in its working. It keeps on printing undesired and redundant results due to the for loop that iterates over the map. I tried a lot of solutions but failed. Is it feasible without using any help of a slice of keys?

package main

import "fmt"

func fizzbuzz(i int)  {
    myMap:= make(map[int]string)
    myMap[3] = "fizz"
    myMap[5] = "buzz"
    myMap[15] = "fizzbuzz"

    for k,v:= range myMap{
        if i%k==0 {fmt.Printf("%v \n",v)
        } else {fmt.Printf("%v \n",i)}
    }
}

func main() {

    for i:=1;i<10000;i++ {
        fizzbuzz(i)
    }

}
icza
  • 389,944
  • 63
  • 907
  • 827
Juan
  • 11
  • 2
  • Please provide the relevant code in the question. Just a link to the source base is not allowed. And ask something specific about your issue. – Mario Santini Feb 16 '17 at 07:42

2 Answers2

1

With a map

With your rule set, the entire for loop should be to decide if the i number is to be replaced with a word. But you emit a result in each iteration. At most one result should be emitted by the for. If i is not dividable by any of the keys, then i should be emitted.

Keys may be multiples of others (e.g. 15 = 3 * 5), and if the i number is dividable by such a key, we want to emit the word associated with the greatest key. So the for loop should not emit anything, because if you find a good key, there may be a greater one. So the loop should just find the greatest good key.

After the loop you can check if any good key was found, and if so, emit the word associated with it, else emit the number:

var rules = map[int]string{
    3:  "fizz",
    5:  "buzz",
    15: "fizzbuzz",
}

func fizzbuzz(i int) {
    max := -1
    for k := range rules {
        if i%k == 0 && k > max {
            max = k
        }
    }

    if max < 0 {
        fmt.Println(i)
    } else {
        fmt.Println(rules[max])
    }
}

func main() {
    for i := 1; i < 100; i++ {
        fizzbuzz(i)
    }

}

Output (try it on the Go Playground):

1
2
fizz
4
buzz
fizz
7
8
fizz
buzz
11
fizz
13
14
fizzbuzz
16
17
fizz
19
buzz
fizz
...

With an ordered slice

You can get better performance if the rules are sorted by the keys descending, in which case you can check the keys in that order (greatest first), and then the first that qualifies will be the greatest. So you can emit the result immediately, and return.

If execution continues after the loop, we know no keys were good, we can emit the i number:

var rules = []struct {
    n    int
    word string
}{
    {15, "fizzbuzz"},
    {5, "buzz"},
    {3, "fizz"},
}

func fizzbuzz(i int) {
    for _, rule := range rules {
        if i%rule.n == 0 {
            fmt.Println(rule.word)
            return
        }
    }

    fmt.Println(i)
}

Try this on the Go Playground.

General (excluding multiples from rules)

Although you started with a rule set where 15 = 3 * 5 was included in the rules, this should not be the case; you should only list 3 and 5, 15 should be implicit.

In this case, you have to check all the rules of course, because each good key should emit a word. And you have to remember if a good key was found, and only emit the i number otherwise.

This is how you can do it:

var rules = []struct {
    n    int
    word string
}{
    {3, "fizz"},
    {5, "buzz"},
}

func fizzbuzz(i int) {
    found := false
    for _, rule := range rules {
        if i%rule.n == 0 {
            found = true
            fmt.Print(rule.word)
        }
    }
    if !found {
        fmt.Print(i)
    }
    fmt.Println()
}

Try it on the Go Playground.

Note: in this solution you could also use a map instead of the slice; the reason why I used a slice is so that in case of multiple good keys the emitted words will always be in the same order (defined by increasing keys), as iteration order of keys in a map is not defined. For details, see Why can't Go iterate maps in insertion order?

Community
  • 1
  • 1
icza
  • 389,944
  • 63
  • 907
  • 827
0

As mentioned, the order of items in a map, is not deterministic in Go. Though here are some simple solutions:

func fizzbuzz(n int) {
    for i := 1; i <= n; i++ {
        switch {
        case i%15 == 0:
            println("fizzbuzz")
        case i%5 == 0:
            println(`buzz`)
        case i%3 == 0:
            println(`fizz`)
        default:
            println(i)
        }
    }
}

func fizzbuzzList(n int) []string {
    var res []string
    for i := 1; i <= n; i++ {
        switch {
        case i%15 == 0:
            res = append(res, `fizzbuzz`)
        case i%5 == 0:
            res = append(res, `buzz`)
        case i%3 == 0:
            res = append(res, `fizz`)
        default:
            res = append(res, strconv.Itoa(i))
        }
    }
    return res
}

func fizzbuzzLazy(n int) chan string {
    var res = make(chan string)
    go func() {
        for i := 1; i <= n; i++ {
            switch {
            case i%15 == 0:
                res <- `fizzbuzz`
            case i%5 == 0:
                res <- `buzz`
            case i%3 == 0:
                res <- `fizz`
            default:
                res <- strconv.Itoa(i)
            }
        }
        close(res)
    }()
    return res
}

And usage:

fizzbuzz(20)

for _, v := range fizzbuzzList(20) {
    println(v)
}

for v := range fizzbuzzLazy(20) {
    println(v)
}
Kaveh Shahbazian
  • 13,088
  • 13
  • 80
  • 139