生成数字时代码进入无限循环

I was trying to write some code that will generate seven sets of seven unique, non-repeated numbers ranging from 1 to 49. However, when I ran the code, it goes into an infinite loop of generating a single number endlessly. This only occurred when I included the little if-else loop under my main() whose functionality is to check for duplicate numbers.

Is there an issue with the logic behind the duplicate check?

package main

import "fmt"
import "math/rand"
import "time"

func main() {

    var j [7]int
    var n []int

    rand.Seed(time.Now().UTC().UnixNano())

    for m := 0; m < 7; m++ {
        for i := 0; i < 8; i++ {
            if i < 7 {
                var duplicate int = randInt(1, 49)
                n = append(n, duplicate)
                if i != 0 {
                    if !integerinarray(duplicate, n) {
                        j[i] = duplicate
                    } else {
                        i--
                    }
                } else {
                    j[i] = duplicate
                }
                fmt.Print(j[i], " ")
            } else {
                fmt.Println("
")
                //fmt.Println(n)
            }
        }
    }

}

func randInt(min int, max int) int {
    return min + rand.Intn(max-min)
}

func integerinarray(a int, s []int) bool {
    for _, b := range s {
        if b == a {
            return true
        }
    }
    return false
}

I modified your program a bit, and now it does what you want in the question.


Code

random_arr.go:

// distribute 1 ~ 49 to 7 group of arrays, each array contain 7 numbers,
// each element should appear one and only once,
package main

import (
    "fmt"
    "math/rand"
    "time"
)

func init() {
    rand.Seed(time.Now().UTC().UnixNano())
}

func main() {
    var arr [7]int
    var used []int

    for i := 0; i < 7; i++ {
        for j := 0; j < 7; {
            for {
                x := randInt(1, 50)         // generate a random number,
                if !checkInSlice(x, used) { // new unique,
                    arr[j] = x
                    j++
                    used = append(used, x)
                    break
                }
            }
        }
        fmt.Printf("[%d] array: %v
", i, arr)
    }

}

// generate random number in range [min, max),
func randInt(min int, max int) int {
    return min + rand.Intn(max-min)
}

// check whether a number is in a slice,
func checkInSlice(a int, s []int) bool {
    for _, b := range s {
        if b == a {
            return true
        }
    }
    return false
}

Execute:

go run random_arr.go

Output:

[0] array: [19 24 47 9 26 21 25]
[1] array: [43 8 27 45 48 16 1]
[2] array: [22 42 31 15 28 39 40]
[3] array: [33 35 11 44 14 36 20]
[4] array: [17 10 7 4 12 6 5]
[5] array: [46 32 13 2 30 49 18]
[6] array: [3 37 34 29 41 38 23]

Tips

About the changes:

  • Change the logic of if/else, use another for loop to do the trick (it's actually more like a while() from C or Java).
    Your original version is unnecessarily complex, and hard to understand.
  • The naming of variable and function are changed to be more easier to read.
  • About rand.Intn(), if you want the range of number to be [0, 49], then need to pass 50, not 49.
  • In fmt.Printf(),You can print an array or slice easily with %v or %#v place holder.
  • Move the seed setting part to init() which is called before the main() automatically, (This is optional).

Possible further improvements:

  • A better algorithm could be designed, so that it would be still efficient for large range of numbers, e.g 0 ~ 1000000.
    The current time complexity is O(n^2), I guess a O(n) algorithm exists.
  • Make the code general & reusable, by refactoring functions.

first of all, you need to format your code. and your logic isn't right.

change to

if integerinarray(duplicate, n) {

you need to debug your code.

A few things I have notice

  • Mixing logic with "logic for printing things" is a bad idea, try to detach that (else {fmt.Println(" ")})
  • I wouldn't recommend to change iterator value inside a for loop (i--)
  • I would avoid to have too much nested levels, make a generateSet function
  • Use slices, you can make them to have a fixed capacity (var j [7]int)

There are more go best practices here https://talks.golang.org/2013/bestpractices.slide

An example solution https://play.golang.org/p/lhnVsYjnei_t

Is there a reason you are not using methods from rand package? ie:

package main

import (
    "math/rand"
    "time"
    "fmt"
)

func main() {
    rand.Seed(time.Now().UnixNano())
    randomInts := rand.Perm(49) 
    // you can also rand.Shuffle an existing set in version 1.10+

    var j [7][7]int

    for i, v := range randomInts {
        j[i/7][i%7] = v + 1
    }
    for i := 0; i < 7; i++ {
        fmt.Println(j[i])
    }
}