1
\$\begingroup\$
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.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

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!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.