2
\$\begingroup\$

I'm just playing about with Go, and am trying to solve some project euler problems to get used to it.

I've created a generator function for Pythagorean triples, using a channel which returns a set of 3 integers for the sides, like so (HCF implements Euclid's algorithm for finding greatest common divisor or highest common factor in this part of the world):

package main
import "fmt"

func HCF(a, b int) int {
    if a % b == 0 {
        return b
    } else {
        return HCF(b, a%b)
    }
}


func PythagTriple() chan [3]int {

    ch := make(chan [3]int)

    go func() {
        for m := 2; ; m++ {
            for n := m%2 + 1; n < m; n += 2 {
                if HCF(m, n) == 1 {
                    var triple [3]int
                    triple[0] = m*m - n*n
                    triple[1] = 2 * m * n
                    triple[2] = m*m + n*n
                    ch <- triple
                }
            }
        }
    }()

    return ch
}

func main() {

    ch := PythagTriple()
    for i := 0; i < 10; i++ {
        fmt.Println(<- ch)
    }
}

Questions:

  1. Do I need to do anything with the channel, or will it close and clean up when it goes out of scope?
  2. Is there a better way of creating a quick array than how I've done it?
  3. Is this idiomatic Go, or am I barking up the wrong tree entirely? I've just started and am not sure if I'm doing it right!

Don't worry too much about efficiency of the algorithm; I'm not too concerned about speed at this point.

\$\endgroup\$
1
\$\begingroup\$
  1. With respect to cleaning up the channel:

    As-is your goroutine will never exit and release it's reference to the channel so it will never get garbage collected. That's fine for something like this where main reads what it wants and then exits. If you want the ability to stop it you can use a stop signal/channel (e.g. see the Go blog Pipeline and/or Context articles.

  2. Creating the array:

    Already answered, ch <- [3]int{x, y, z} is likely the most concise way and is quite reasonable. See also Composite Literals in the spec for when you want to this with a struct instead of an array/slice.

  3. Idiomatic Go:

    It look okay to me (except for gofmting). Run gofmt, golint, and go vet on your code (go fmt all the time and before check-in, the others as required and consider the output as recommendations).

    Also (re-)read Effective Go and the Go Author code review comments (the later should probably be read more as a recomendation rather than rules, but I prefer to try and follow it).

\$\endgroup\$
  • \$\begingroup\$ Thank you very much for the suggestions - particularly the hints about releasing the channel for garbage collection, suspected I wasn't quite getting that bit right! \$\endgroup\$ – Andrew Charlton Mar 4 '15 at 21:40
1
\$\begingroup\$

Not a Go expert but I think you should be able to define your array of values as you declare it :

triple := [...]int{m*m - n*n, 2 * m * n, m*m + n*n}

Source

\$\endgroup\$
  • \$\begingroup\$ This I think is one of the bigger improvements overall (and gets rid of the var triple [3]int declaration. \$\endgroup\$ – jsanc623 Mar 2 '15 at 15:30
  • \$\begingroup\$ Thank you - have just skimmed through the docs so far, must have missed that way of declaring arrays, that's much clearer. \$\endgroup\$ – Andrew Charlton Mar 2 '15 at 16:05
0
\$\begingroup\$

Using Josay's suggestion then, is this the cleanest way to do it?

package main
import "fmt"

func HCF(a, b int) int {
    if a % b == 0 {
        return b
    } else {
        return HCF(b, a%b)
    }
}


func PythagTriple() chan [3]int {

    ch := make(chan [3]int)

    go func() {
        for m := 2; ; m++ {
            for n := m%2 + 1; n < m; n += 2 {
                if HCF(m, n) == 1 {
                    ch <- [3]int{m*m + n*n, 2*m*n, m*m - n*n}
                }
            }
        }
    }()

    return ch
}

func main() {

    ch := PythagTriple()
    for i := 0; i < 10; i++ {
        fmt.Println(<- ch)
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Self-answers should also incorporate specific changes made. If you'd like further review instead, post a new follow-up question. \$\endgroup\$ – Jamal Mar 2 '15 at 16:23

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.