Code review comment for lp:~sidnei/juju-core/sort-by-cost-lowest-fallback

Please take a look.

https://codereview.appspot.com/12569044/diff/6001/environs/instances/instancetype.go
File environs/instances/instancetype.go (right):

https://codereview.appspot.com/12569044/diff/6001/environs/instances/instancetype.go#newcode131
environs/instances/instancetype.go:131: if bc[i].Cost != bc[j].Cost {
On 2013/08/08 15:07:24, rog wrote:
> One way to make this slightly more readable
> (and more efficient into the bargain) is
> to use pointers to the two elements in question:

> inst0, inst1 := &bc[i], &bc[j]

> then:

> if inst0.Cost != inst0.Cost {
> return inst0.Cost < inst1.Cost
> }

> etc

Done.

https://codereview.appspot.com/12569044/diff/6001/environs/instances/instancetype.go#newcode137
environs/instances/instancetype.go:137: iCpuPower := uint64(0)
On 2013/08/08 15:07:24, rog wrote:
> or:
> var iCpuPower, jCpuPower int64

Done.

https://codereview.appspot.com/12569044/diff/6001/environs/instances/instancetype.go#newcode139
environs/instances/instancetype.go:139: if bc[i].CpuPower != nil {
On 2013/08/08 15:07:24, rog wrote:
> I don't think either of these should be compared
> at all if they're nil.

> So something like this perhaps?

> if inst0.CpuPower != nil &&
> inst1.CpuPower != nil &&
> *inst0.CpuPower != *inst1.CpuPower{
> return *inst0.CpuPower < *inst1.CpuPower
> }

Done.

https://codereview.appspot.com/12569044/diff/6001/environs/instances/instancetype_test.go
File environs/instances/instancetype_test.go (right):

https://codereview.appspot.com/12569044/diff/6001/environs/instances/instancetype_test.go#newcode287
environs/instances/instancetype_test.go:287: },
On 2013/08/08 15:07:24, rog wrote:
> I'd like a test for a case where cpu power is only set in one of the
instances.

Done.

https://codereview.appspot.com/12569044/