Modelling employees in Go as a basic OOP exercise

Question

I am learning Go and have the following working code:

$GOPATH/src/mvctest/main.go

package main

import (
    "fmt"
    "mvctest/model"
)

func main() {
    // Initialise the data model
    employees := model.NewPeople("Employee")

    // Populate the model
    employees.Add(model.NewPerson("Joe", 99))
    employees.Add(model.NewPerson("Sue", 45))
    employees.Add(model.Person{Name: "Tom", Age: 22}) // break encaps a bit

    // Test retrieval from model
    fmt.Printf("You have %d employees.\n", employees.Count())

    for i := 0; i < employees.Count(); i++ {
        f, err := employees.Person(i)
        if err != nil {
            fmt.Printf("Unable to get employee %d because %v", i, err)
            return
        }
        fmt.Printf("Employee %d is %s\n", i+1, f)
    }

    // Test error handling for impossible values
    test(employees, -1)
    test(employees, 42)

}

// Tests whether a person can be retrieved using an index value
func test(ppl *model.People, i int) *model.Person {
    p, err := ppl.Person(i)
    if err != nil {
        fmt.Printf("Unable to get %s %d because %v\n", ppl.Group(), i, err)
        return nil
    }
    return p
}

$GOPATH/src/mvctest/model/model.go

package model

import (
    "errors"
    "fmt"
)

// ------------------------------------------------------

type Person struct {
    Name    string
    hatsize int
    Age     int
}

func (p *Person) String() string {
    return fmt.Sprintf("%s aged %d.", p.Name, p.Age)
}

func NewPerson(name string, age int) Person {
    return Person{name, 0, age}
}

// ------------------------------------------------------

type People struct {
    group string
    list  []Person
}

func (ppl *People) SetGroup(g string) {
    ppl.group = g
}

func (ppl *People) Group() string {
    return ppl.group
}

func (ppl *People) Add(p Person) {
    ppl.list = append(ppl.list, p)
}

func (ppl *People) Count() int {
    return len(ppl.list)
}

func (ppl *People) Person(i int) (*Person, error) {
    n := len(ppl.list)
    if i < 0 || i >= n {
        return nil,
            errors.New(fmt.Sprintf("%s index %d is out of range %d-%d.",
                ppl.group, i, 0, len(ppl.list)-1))
    }
    return &ppl.list[i], nil
}

func NewPeople(group string) *People {
    ppl := new(People)
    ppl.group = group
    return ppl
}

Output

You have 3 employees.
Employee 1 is Joe aged 99.
Employee 2 is Sue aged 45.
Employee 3 is Tom aged 22.
Unable to get Employee -1 because Employee index -1 is out of range 0-2.
Unable to get Employee 42 because Employee index 42 is out of range 0-2.

I'd appreciate any comments but a few things that I am uncertain about or which bother me.

• I'm not sure whether defining constructing NewPerson and NewPeople is the right thing to do. I can't have two functions called New in the same file / package and I can't make these into methods of different structs (i.e. different object types) since the caller doesn't yet have an instance to use.
• I'm wondering if there is a simple way to allow iterating using range rather than the for loop in main.go?

Answer 1

Using a NewType() constructor-like function is the idiomatic way in Go. See Effective Go: Constructors and composite literals. Also relevant: Effective Go: Package names:

The importer of a package will use the name to refer to its contents, so exported names in the package can use that fact to avoid stutter. (Don't use the import . notation, which can simplify tests that must run outside the package they are testing, but should otherwise be avoided.) For instance, the buffered reader type in the bufio package is called Reader, not BufReader, because users see it as bufio.Reader, which is a clear, concise name. Moreover, because imported entities are always addressed with their package name, bufio.Reader does not conflict with io.Reader. Similarly, the function to make new instances of ring.Ring—which is the definition of a constructor in Go—would normally be called NewRing, but since Ring is the only type exported by the package, and since the package is called ring, it's called just New, which clients of the package see as ring.New. Use the package structure to help you choose good names.

And if you want to iterate over a whole "collection" (e.g. slice, array, map, channel), the For statements with range clause is the clearest way. "Unfortunately" your People type does not expose such values, so it's not possible to use for range here.

It can still be improved to call People.Count() once:

for i, count := 0, employees.Count(); i < count; i++ {
    f, err := employees.Person(i)
    // ...
}

Other improvements

Your NewPeople() could be simply written as:

func NewPeople(group string) *People {
    return &People{group: group}
}

In People.Person() it's redundant to call errors.New() and fmt.Sprintf(), as the fmt package also has an fmt.Errorf() function doing just that:

func (ppl *People) Person(i int) (*Person, error) {
    if n := len(ppl.list); i < 0 || i >= n {
        return nil, fmt.Errorf("%s index %d is out of range %d-%d.",
            ppl.group, i, 0, n-1)
    }
    return &ppl.list[i], nil
}

Also it's just a matter of taste, but I find it easier to read if you use the max index instead of the length (which is max+1):

if max := len(ppl.list) - 1; i < 0 || i > max {
    return nil, fmt.Errorf("%s index %d is out of range %d-%d.",
        ppl.group, i, 0, max)
}

Also it's a good habit for a specific type to be consistent. Either use pointers in all places, or non-pointers. In case of Person, you mix both: People.Add() expects Person, but People.Person() returns *Person.