-2

I am writing a simple game in Go and have some problems with it. My, cut off, code looks like this:

package main

import "fmt"

type Location struct {
    X int
    Y int
}

type Car struct {
    MaxSpeed int
    Loc Location
}

func (car Car) SetLocation(loc Location) {
    car.Loc = loc
}

func (car Car) GetLocation() Location {
    return car.Loc
}

type Bike struct {
    GearsNum int
    Loc Location
}

func (bike Bike) SetLocation(loc Location) {
    bike.Loc = loc
}

func (bike Bike) GetLocation() Location {
    return bike.Loc
}
type Movable interface {
    GetLocation() Location
    SetLocation(Location)
}

type Fleet struct {
    vehicles []Movable
}

func (fleet *Fleet) AddVehicles(v ...Movable) {
    for _, x := range(v) {
        fleet.vehicles = append(fleet.vehicles, x) 
    }
}

func (fleet *Fleet) WherTheyAre() {
    for _, v := range(fleet.vehicles) {
        fmt.Println(v.GetLocation())
    }
}

func main() {
    myCar := Car{MaxSpeed: 200, Loc: Location{12, 34}}
    myBike := Bike{GearsNum: 11, Loc: Location{1, 1}}

    myFleet := Fleet{}
    myFleet.AddVehicles(myCar)
    myFleet.AddVehicles(myBike)
    myFleet.WherTheyAre()

    myCar.SetLocation(Location{0,0})
    myFleet.WherTheyAre()
}

The assumption is that Car and Bike are very big structures which I do not want to copy. How should I design the code to be able to modify the location of the car which is the part of the Fleet? Other words how to design the Fleet struct to be able to modify its movable objects? I have tried to experiment with pointers to interfaces but I was not good idea...

Thanks for help!

mmburr
  • 7
  • 2
  • 4
    Your methods on `Car` and `Bike` need pointer receivers, not value receivers. –  Jun 11 '15 at 22:02
  • That is not the point. I am asking how to have a "handler" thanks to which I will be able to modify vehicles in Fleet. Func: AddVehicles will copy it and I can not change its argument type to pointer because the type is interface. – mmburr Jun 11 '15 at 22:18
  • Simply pass a pointer to `AddVehicles`: `myFleet.AddVehicles(&myCar); myFleet.AddVehicles(&myBike)`; you do not have to (and should not) change `vehicles []Movable` to `vehicles []*Movable`. –  Jun 11 '15 at 22:21
  • It works... Thank you. But my mind just blow up. How it can work if the types are not correct? – mmburr Jun 11 '15 at 22:29
  • See: http://stackoverflow.com/questions/13511203/why-cant-i-assign-a-struct-to-an-interface –  Jun 11 '15 at 22:34

1 Answers1

1

The problem is that you've defined the method with value receivers. When you call a method on a receiver the receiving type is actually being passed as an argument, in this case by value and you're modifying that copy.

func (bike Bike) SetLocation(loc Location) {
    bike.Loc = loc // modifies local copy of Bike
}


func (bike *Bike) SetLocation(loc Location) {
        bike.Loc = loc // modifies the referenced bike
    }

You gotta declare your types differently though, or use the & operator in order to call these methods because you have value types in your main. My preference is to use this syntax bike := &Bike{} in almost all cases. I will only go away from it if I have a convenience method for initilization or a very specific reason for using a value type (which is extremely rare).

But basically, you can't make a setter use a value type receiver. If you want to use these structs as value types I would recommend exporting the fields so they can be accessed without getter or setter. Also, just regarding style, I would be displeased to see the use of getters and setters at all unless you actually are abstracting some logic. There's no point in not exporting a field if you're going to provide a setter that directly assigns the value passed in to said field. Also, wasn't looking to closely but you are exporting all fields on your structs so your setters are useless and most programmers would just do bike.Loc = anythingIWantBecauseThisFieldsExportedWhichYouMayThinkOfAsPublic

evanmcdonnal
  • 46,131
  • 16
  • 104
  • 115
  • please see the comment above. Regarding the getters and setter I agree. They do not make sense I just wanted to have interface. Maybe I should figure out sth more convenient. – mmburr Jun 11 '15 at 22:19
  • @mmburr I see. I think I'm going to leave this as is though case the solution you're going with is more of a refactor/redesign than the answer to why you weren't able to modify those fields. If you're not actually using what Tim proposed in the comments please speak up and I'll help you get a working solution. – evanmcdonnal Jun 11 '15 at 23:28