snippetgoMinor
Implementation of merge sort and bubble sort
Viewed 0 times
mergebubblesortandimplementation
Problem
I'm new to Go and looking for a review of my merge sort and bubble sort implementations. What can be done better? Can my code be cleaner/clearer?
// algos.go
// (C) 2014 splashinn
// Implement two types of sorting algorithms; merge sort and bubble sort
package main
import (
"fmt"
"time"
"math/rand"
)
func main() {
var n int
fmt.Printf("How long of an array should we sort? (≤1000) ")
_, err := fmt.Scanf("%d", &n)
if err != nil {
fmt.Println("Error:", err)
return
}
if n > 1000 {
fmt.Println("Error:", n, "> 1000 - too long!")
return
} else if n array[j+1] {
// Idiomatic Go swap
array[j], array[j+1] = array[j+1], array[j]
}
}
}
return array
}
// Implementing a top-down merge sort
// Adapted from the Merge Sort C code from Wikipedia
func MergeSort(array []int) (sortedArray []int) {
sortedArray = make([]int, len(array), cap(array))
splitMerge(array, sortedArray)
return array
}
func splitMerge(A []int, B []int) {
if(len(A) = iEnd || A[i0] <= A[i1])) {
B[j] = A[i0]
i0++ // Increment i0 after using it as an index.
} else {
B[j] = A[i1]
i1++ // Increment i1 after using it as an index.
}
}
}Solution
I don't see any major problems but there are a number of (really very minor) little nits.
First, the trend seems to be for people to like very few blank lines. I think this trend is driven by the prevalence of syntax highlighting. I recently I had the occasion to program on a machine without syntax highlighting and found that I wanted more blank lines again for readability. Anecdotes aside though, try using fewer blank lines and see if you like it after you get used to it.
In your first if statement, you could use the "simple statement" form
Most Go programmers like that this form more clearly associates the if statement with the code that is being tested and that the scope of
Consider replacing sequences of if statements with a single switch statement.
The usual arguments for preferring switch over a chain of ifs are that the logic is easier to follow and the code is more readable. As with the simple statement form of if, this form also restricts variable scope and tends to need few lines on the screen.
The word "array" has a general meaning in computing, but has a specific meaning in Go. Some Go programmers are bothered when Go slices are incorrectly called arrays. It's disturbing in the way something like
In BubbleSort, you can replace
with
if you like. The first form is easy to understand and is arguably more readable. The second form is a little tricky but is listed on Slice Tricks on the Go Wiki and I think is pretty common.
It looks like your for loops in BubbleSort could use the range clause form.
And then you have some if statements with unnecessary parentheses. Otherwise looks great.
First, the trend seems to be for people to like very few blank lines. I think this trend is driven by the prevalence of syntax highlighting. I recently I had the occasion to program on a machine without syntax highlighting and found that I wanted more blank lines again for readability. Anecdotes aside though, try using fewer blank lines and see if you like it after you get used to it.
In your first if statement, you could use the "simple statement" form
if _, err := fmt.Scanf("%d", &n); err != nil {
fmt.Println("Error:", err)
return
}Most Go programmers like that this form more clearly associates the if statement with the code that is being tested and that the scope of
err here is limited to the if body. It also saves a line of screen real estate.Consider replacing sequences of if statements with a single switch statement.
switch _, err := fmt.Scanf("%d", &n); {
case err != nil:
fmt.Println("Error:", err)
return
case n > 1000:
fmt.Println("Error:", n, "> 1000 - too long!")
return
case n < 1:
fmt.Println("Error:", n, "< 1")
return
}The usual arguments for preferring switch over a chain of ifs are that the logic is easier to follow and the code is more readable. As with the simple statement form of if, this form also restricts variable scope and tends to need few lines on the screen.
array := make([]int, n) presents some issues.The word "array" has a general meaning in computing, but has a specific meaning in Go. Some Go programmers are bothered when Go slices are incorrectly called arrays. It's disturbing in the way something like
var integer float64 would be. You might still argue that array is appropriate here because it identifies the conceptual thing you have obtained from the user and retains the language of the user interface. Or in the interest of code readability, you might consider other names, or consider different terminology in the user interface.In BubbleSort, you can replace
array := make([]int, len(inputArray))
copy(array, inputArray)with
array := append([]int{}, inputArray...)if you like. The first form is easy to understand and is arguably more readable. The second form is a little tricky but is listed on Slice Tricks on the Go Wiki and I think is pretty common.
It looks like your for loops in BubbleSort could use the range clause form.
And then you have some if statements with unnecessary parentheses. Otherwise looks great.
Code Snippets
if _, err := fmt.Scanf("%d", &n); err != nil {
fmt.Println("Error:", err)
return
}switch _, err := fmt.Scanf("%d", &n); {
case err != nil:
fmt.Println("Error:", err)
return
case n > 1000:
fmt.Println("Error:", n, "> 1000 - too long!")
return
case n < 1:
fmt.Println("Error:", n, "< 1")
return
}array := make([]int, len(inputArray))
copy(array, inputArray)array := append([]int{}, inputArray...)Context
StackExchange Code Review Q#54996, answer score: 3
Revisions (0)
No revisions yet.