HiveBrain v1.2.0
Get Started
← Back to all entries
patterngoMinor

Golang implementation of Pascal's triangle

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pascalimplementationgolangtriangle

Problem

How can I improve my code and make it more idiomatic?

package pascal

func NextRow(prevRow []int) []int {
    x := make([]int, len(prevRow)+1, len(prevRow)+1)
    x[0], x[len(prevRow)] = 1, 1
    for i := 0; i < len(prevRow)-1; i++ {
        x[i+1] = prevRow[i] + prevRow[i+1]
    }
    return x
}

func Triangle(x int) [][]int {
    y := [][]int{}
    for i := 0; i < x; i++ {
        if i == 0 {
            y = append(y, []int{1})
        } else {
            y = append(y, NextRow(y[i-1]))
        }
    }
    return y
}

Solution

It's interesting that you chose to put the code in its own package. I think it's great - but it does make the code a little more difficult to run as an experiment, or on ideone, or something. So, I encourage you to use packages, but remember that the main function/entry point will need to be in a different directory.

Now, as a package, there should be no reason to have an exported function NextRow. Why have you capitalized it? It should be nextRow.

Then, working through your functions, the next most striking thing is the poor choice of variable names. x's that are "lines", y's that are "triangles", etc. Worse, you use the variable x in two different ways in the two different functions. Renaming some variables made it easier to read.

Finally, the go fmt tool will re-format your code to conform to go conventions. You should always do this for your code. It helps when sharing go code with others.

With those changes, I would consider your code to be reasonably good.

On the other hand, I did mess a bit with the nextRow method to use append(..) instead of direct index assignment. I feel the append helps by making the 1 at the beginning and end of the line more obvious.

I also moved the special-case of the first row outside the loop of the Triangle method. This makes it clearer, I feel.

So, your major issues are code style, and naming. The actual code is otherwise OK. Some tweaks may help readability - but that's something you should decide whether it's better.

I put those changes together in a runnable format in ideone.

package main

import (
    "fmt"
    "strings"
)

func nextRow(prevRow []int) []int {
    current := make([]int, 0, len(prevRow)+1)

    current = append(current, 1)

    for i := 1; i < len(prevRow); i++ {
        current = append(current, prevRow[i-1]+prevRow[i])
    }

    current = append(current, 1)

    return current
}

func Triangle(rows int) [][]int {
    triangle := [][]int{}
    if rows <= 0 {
        return triangle
    }
    triangle = append(triangle, []int{1})
    for i := 1; i < rows; i++ {
        triangle = append(triangle, nextRow(triangle[i-1]))
    }
    return triangle
}

func present(line []int) string {
    words := make([]string, len(line), len(line))
    for i, v := range line {
        words[i] = fmt.Sprintf("%4v ", v)
    }
    return strings.Join(words, " ")
}

func main() {
    tri := Triangle(10)
    for _, line := range tri {
        fmt.Println(present(line))
    }
}

Code Snippets

package main

import (
    "fmt"
    "strings"
)

func nextRow(prevRow []int) []int {
    current := make([]int, 0, len(prevRow)+1)

    current = append(current, 1)

    for i := 1; i < len(prevRow); i++ {
        current = append(current, prevRow[i-1]+prevRow[i])
    }

    current = append(current, 1)

    return current
}

func Triangle(rows int) [][]int {
    triangle := [][]int{}
    if rows <= 0 {
        return triangle
    }
    triangle = append(triangle, []int{1})
    for i := 1; i < rows; i++ {
        triangle = append(triangle, nextRow(triangle[i-1]))
    }
    return triangle
}

func present(line []int) string {
    words := make([]string, len(line), len(line))
    for i, v := range line {
        words[i] = fmt.Sprintf("%4v ", v)
    }
    return strings.Join(words, " ")
}

func main() {
    tri := Triangle(10)
    for _, line := range tri {
        fmt.Println(present(line))
    }
}

Context

StackExchange Code Review Q#115689, answer score: 4

Revisions (0)

No revisions yet.