Swapping alternate nodes of linkedlist

Question

package main
import (
 "fmt"
)
type LinkedList struct {
 first, last *node 
}
type node struct {
 item int
 next *node
}
func (l *LinkedList) add (val int) {
 n := node {
 item: val,
 next: nil,
 }
 if l.first == nil {
 l.last = &n
 l.first = &n
 } else {
 l.last.next = &n
 l.last = &n
 } 
}
func (l *LinkedList) traverse () {
 for n := l.first; n != nil; n = n.next {
 fmt.Printf("%v ", n.item)
 }
 fmt.Println()
}
func (l *LinkedList) swapLL () {
 if l.first == nil {
 panic("First element cannot be null")
 }
 if l.first.next == nil {
 return
 }
 ptr1 := l.first
 var ptr2 *node
 var ptr3 *node
 var prev *node
 for ; ptr1 != nil && ptr1.next != nil; {
 //Allocate resources
 ptr2 = ptr1.next
 ptr3 = ptr2.next
 //swap
 ptr2.next = ptr1
 ptr1.next = ptr3
 //hook to the previous pair
 if prev == nil {
 l.first = ptr2
 } else {
 prev.next = ptr2
 }
 //advance
 prev = ptr1
 ptr1 = ptr3
 }
}
func main () {
 l := LinkedList {
 first: nil,
 last: nil,
 }
 l.add(10)
 l.add(20)
 l.add(30)
 l.add(40)
 l.traverse()
 l.swapLL()
 l.traverse()
}

---

Input:

Linkedlist: 10 -> 20 -> 30 -> 40 -> null

Output:

Linkedlist: 20 -> 10 -> 40 -> 30 -> null

Question:

Here 10 and 20 are adjacent thus they were swapped. Likewise, 30 and 40 are adjacent thus they were swapped.

I am looking for suggestions for improving my code, any optimizations, or anything to make my code more readable and elegant.

Answer 1

Welcome!

You may be interested in viewing the Go API implementation of a linked list. Their implementation is not restricted to just integer node data.

Use go fmt

A few of your lines have trailing whitespace. Likewise, some of your formatting isn't standard for Go.

If you run go fmt on the source code, it cleans all that up for you.

Don't unnecessarily export things

Your type LinkedList is exported. If this is for a library, you should add a comment explaining it's usage (used to generate documentation: see Godoc). If it's not for a library, it shouldn't be exported.

Going forward, I'll assume it's not meant for a library. Otherwise the fields of LinkedList also need to be exported, and the node type as well.

Avoid extra typing when initializing a struct

When initializing a struct, you don't have to initialize the fields to values that would already be the default.

See §Composite Literals in the language specification for more details.

n := node{
 item: val,
 next: nil,
}

Can be written as:

n := node{
 item: val,
}

And

l := linkedList{
 first: nil,
 last: nil,
}

Becomes

l := linkedList{}

Combine variable declarations:

We can combine multiple variable declarations under one var keyword.

var ptr2 *node
var ptr3 *node
var prev *node

Becomes either

var ptr2, ptr3, prev *node

Or

var (
 ptr2 *node
 ptr3 *node
 prev *node
)

I prefer the shorter notation most times when the variables are of the same type, but either is better.

Use the specific Printf verb

You use the %v verb, but here we know at compile time that we're printing integers. You can safely use %d instead.

Return an error instead of panic()ing

Unless you expect to always recover() from the panic(), you should return an error value instead. This is more common.

func (l *linkedList) swapLL() {
 if l.first == nil {
 panic("First element cannot be null")
 }
 if l.first.next == nil {
 return
 }
 //...
}

Becomes

func (l *linkedList) swapLL() error {
 if l.first == nil {
 return fmt.Errorf("List cannot be empty")
 }
 if l.first.next == nil {
 return nil
 }
 //...
 return nil
}

This now makes it easy to check for the error:

if err := l.swapLL(); err != nil {
 log.Fatal(err)
}

Conclusion

Here is the final source I ended up with.

package main
import (
 "fmt"
 "log"
)
type linkedList struct {
 first, last *node
}
type node struct {
 item int
 next *node
}
func (l *linkedList) add(val int) {
 n := node{
 item: val,
 }
 if l.first == nil {
 l.last = &n
 l.first = &n
 } else {
 l.last.next = &n
 l.last = &n
 }
}
func (l *linkedList) traverse() {
 for n := l.first; n != nil; n = n.next {
 fmt.Printf("%d ", n.item)
 }
 fmt.Println()
}
func (l *linkedList) swapLL() error {
 if l.first == nil {
 return fmt.Errorf("List cannot be empty")
 }
 if l.first.next == nil {
 return nil
 }
 ptr1 := l.first
 var ptr2, ptr3, prev *node
 for ptr1 != nil && ptr1.next != nil {
 //Allocate resources
 ptr2 = ptr1.next
 ptr3 = ptr2.next
 //swap
 ptr2.next = ptr1
 ptr1.next = ptr3
 //hook to the previous pair
 if prev == nil {
 l.first = ptr2
 } else {
 prev.next = ptr2
 }
 //advance
 prev = ptr1
 ptr1 = ptr3
 }
 return nil
}
func main() {
 l := linkedList{}
 l.add(10)
 l.add(20)
 l.add(30)
 l.add(40)
 l.traverse()
 if err := l.swapLL(); err != nil {
 log.Fatal(err)
 }
 l.traverse()
}

Hope this helps!