0
\$\begingroup\$

I was trying to implement visitor pattern in Golang, and came across an idea of using a generic func type instead if interface as visitor. Here is my code:

package main

import (
    "encoding/json"
    "encoding/xml"
    "fmt"
)

type Visitor func(shape Shape)

type Shape interface {
    accept(Visitor)
}

type Circle struct {
    Rad int
}

func (c Circle) accept(v Visitor) {
    v(c)
}

type Line struct {
    Length int
}

func (l Line) accept(v Visitor) {
    v(l)
}

func JsonVisitor(shape Shape) {
    bytes, err := json.Marshal(shape)
    if err != nil {
        panic(err)
    }
    fmt.Println(string(bytes))
}

func XmlVisitor(shape Shape) {
    bytes, err := xml.Marshal(shape)
    if err != nil {
        panic(err)
    }
    fmt.Println(string(bytes))
}

func main() {
    circle := Circle{12}
    line := Line{42}
    circle.accept(JsonVisitor)
    line.accept(JsonVisitor)

    circle.accept(XmlVisitor)
    line.accept(XmlVisitor)
}

Besides the code review I have a question: is it still considered a Visitor pattern if using one generic function for all the Shapes?

\$\endgroup\$
2
\$\begingroup\$

General things:

  • The visitor pattern is more suited for object-oriented languages. Whether Go is object-oriented is up for debate, but fact is that many OOP patterns are implemented differently in it (or don't make sense at all).
  • Specifically, I don't see an advantage to XmlVisitor and JsonVisitor over xml.Marshal and json.Marshal. That said, design is beyond the scope of codereview SE, so I'll assume you have a good reason.
  • Create a separate package rather than adding to main. I understand this is something you'd do eventually, but it's good practise to do it from the very start.

type Visitor func(shape Shape)

Do you actually need this to be a type? I see little advantage to it (slightly shorter lines perhaps) and this effectively hides from the user what Visitor really is. func (c Circle) accept(v func(Shape)) is self-documenting and easier to read. If you must have your own type, name it VisitorFunc at least, to make it clear what it is.


type Shape interface {
    accept(Visitor)
}

Probably not the best name - it can be anything, doesn't neccessarily need to be shape. I'd go with e.g. "Visitable", but the proper name per Effective Go would be "Accepter" since it contains a single method (use your best judgement to decide which name is better).

More importantly, accept shouldn't be private - you won't be able to call private methods outside of the package where the type is defined.


func (c Circle) accept(v Visitor) {
    v(c)
}

You probably want to pass by reference (pointer): func (c *Circle) accept.... The way it is now, your visitor is read-only and also more costly, as the whole object is copied.


func JsonVisitor(shape Shape) {
    bytes, err := json.Marshal(shape)
    if err != nil {
        panic(err)
    }
    fmt.Println(string(bytes))
}

func XmlVisitor(shape Shape) {
    bytes, err := xml.Marshal(shape)
    if err != nil {
        panic(err)
    }
    fmt.Println(string(bytes))
}

Don't panic(...). panic is used for unrecoverable errors - you'll only use it if you can answer "yes" to "am I confident that, whenever this errors happens, it's so severe that the best thing to do is just crash". Instead, have your accept method, your Visitor type and these functions all return error.

\$\endgroup\$
  • \$\begingroup\$ Design is certainly within the scope of Code Review, if you wish to discuss it further. \$\endgroup\$ – 200_success Oct 19 '17 at 13:44
  • \$\begingroup\$ There are no advantages of this implementation over the go stdlib marshallers. This is just a example of an implementation of visitor pattern in go. So the goal is to show what these pattern is useful for. Thank you for a lot of useful remarks! \$\endgroup\$ – Sergii Bishyr Oct 19 '17 at 14:01

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.