39

I keep getting this error when trying to range over a slice pointer.

app/domain/repositories/class_repository.go:24: cannot range over classes (type *[]entities.Class)

What am I doing wrong?

Here is the struct:

 package repositories

import (
    "mobifit/app/domain/entities"
)

type ClassRepository struct {
    *Repository
}

func (c *ClassRepository) ClassesForLastNDays(days int) *[]entities.Class {
    classes := new([]entities.Class)
    query := Select("*").
        From("Class").
        Where("VisibleAt > CURRENT_TIMESTAMP() - INTERVAL ? DAY").
        OrderBy("ClassTypeId").
        Sql()
    c.Repository.Select(classes, query, days)
    c.populateClassRelationships(classes)
    return classes
}

func (c *ClassRepository) populateClassRelationships(classes *[]entities.Class) {
    for i := range classes {  <<<<<<<<<<< Here is the problem
        class := classes[i]

        // ClassType
        c.Repository.GetById(class.ClassType, class.ClassTypeId)

        //Instructor
        c.Repository.GetById(class.Instructor, class.ClassType.InstructorId)

        // Equipment
        query := Select("E.*").
            From("Equipment E").
            Join("ClassEquipment CE on E.Id = CE.EquipmentId").
            Where("CE.ClassId = ?").
            Sql()
        c.Repository.Select(class.Equipment, query, class.Id)
    }
}

Here is the Class struct:

package entities

import (
    "time"
)

    type Class struct {
        Id                int
        ClassTypeId       int
        VideoPath         string
        VideoSize         int
        Duration          float64
        CreatedAt         time.Time
        VisibleAt         time.Time
        NoLongerVisibleAt time.Time

        // Relationships
        ClassType  ClassType
        Instructor User
        Equipment  []Equipment
    }
Improve this question
4
  • A slice is already a kind of pointer, there's no reason to point to it. Commented Jan 22, 2014 at 8:45
  • I wanted to have a slice of pointers, so I can populate them with the PoulateClassRelationships func Commented Jan 22, 2014 at 8:54
  • @dystroy I think you actually have the best answer now, as you have actually gotten to the root of the problem. According to golang.org/doc/effective_go.html#slices, If a function takes a slice argument, changes it makes to the elements of the slice will be visible to the caller, analogous to passing a pointer to the underlying array Commented Jan 22, 2014 at 9:31
  • 1
    I have once stepped on the same rake. Play. Commented Sep 26, 2014 at 17:55

4 Answers 4

Reset to default
40

You're assuming the pointer to a slice will be automatically dereferenced for the iteration.

That's not the case and there's no reason for that because a slice is already a kind of pointer, rendering a pointer to a slice totally useless.

From Effective Go :

If a function takes a slice argument, changes it makes to the elements of the slice will be visible to the caller, analogous to passing a pointer to the underlying array.

Internally, a slice is made of

  • a pointer to the first element of the slice in the underlying array
  • the length of the slice
  • the capacity of the slice (the slice can usually be extended until the end of the array)

This structure is very small, rendering a pointer useless.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Image
Clarification: There is one use for pointer to a slice: If multiple sections of the program need to share the same slice, so modifications to the slice itself are reflected in other sections of the program (for instance, if removing an element from a slice by doing a = append(a[:i], a[i+1:]...) should be reflected in slices held by other data structures). This is rarely what you want, however, and is not thread-safe without locking.
Image
Just spent hours debugging as I was trying to be too clever. I had a function that was returning a []*structs as I didn't want to duplicate the memory. This wreaked havoc on my code in a for each loop when building the return result as every pointer in my slice pointed to the memory address of the for each loop iterator which means that the slice that I ended up returning had 10 pointers all pointing to the same struct. When I took pointers out of the equation and just returned a normal slice after my for each loop everything worked perfectly fine.
14

if you need to pull an individual element from the *slice, you have to dereference it first like this: (*slice)[0]. I pounded my head against *slice[0] for about 6 hours before I realized this. It has to do with the order of operations, and is not, IMO, a very elegant result.

I ended up writing some pointer receiver methods to do in-place modifications like append and pop in a more, to my mind, reasonable way - an example can be found here: https://play.golang.org/p/qZEYMcPHl4

Improve this answer

1 Comment

Image
@HassaanSalik - thanks for the upvote, so I'll pass along that I don't ever do this anymore. A slice is already a pointer type, so there usually shouldn't ever be a reason to need a *slice - I've since refactored this code, and I'd recommend you look for opportunities to do the same.
12

From Effective Go:

If you're looping over an array, slice, string, or map, or reading from a channel, a range clause can manage the loop.

You are attempting to iterate over a pointer to a slice which is a single value, not a collection therefore is not possible.

Change the argument to populateClassRelationships to be an slice, not a pointer to a slice. Or you could dereference the pointer:

func (c *ClassRepository) populateClassRelationships(classes *[]entities.Class) {
    for i := range *classes { // dereferencing the pointer to get the actual slice
        class := classes[i]

        // ClassType
        c.Repository.GetById(class.ClassType, class.ClassTypeId)

        //Instructor
        c.Repository.GetById(class.Instructor, class.ClassType.InstructorId)

        // Equipment
        query := Select("E.*").
            From("Equipment E").
            Join("ClassEquipment CE on E.Id = CE.EquipmentId").
            Where("CE.ClassId = ?").
            Sql()
        c.Repository.Select(class.Equipment, query, class.Id)
    }
}
Improve this answer

5 Comments

Image
No, it doesn't have to be a pointer I am just trying to get it to work.
Image
Can you show me how to do it without the pointers? I copied the key part to play.golang.org/p/KonrOk3bp-
Image
There is a problem with line 19.
Image
I thought that if you didn't pass a pointer to the array when calling populateClassRelationships then you are just populating a copy and the original will be blank on return in ClassesForLastNDays
Image
Now getting cannot use &classes (type **[]entities.Class) as type []entities.Class in function argument
5

You could dereference the pointer:

func (c *ClassRepository) populateClassRelationships(classes *[]entities.Class) {
    for _, class := range *classes { // NOTE the * dereference
    // ClassType
    c.Repository.GetById(class.ClassType, class.ClassTypeId)

    //Instructor
    c.Repository.GetById(class.Instructor, class.ClassType.InstructorId)

    // Equipment
    query := Select("E.*").
        From("Equipment E").
        Join("ClassEquipment CE on E.Id = CE.EquipmentId").
        Where("CE.ClassId = ?").
        Sql()
    c.Repository.Select(class.Equipment, query, class.Id)
    }
}

I also changed the range clause as I don't think you're modifying classes.

Improve this answer

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.