State interfaces

[John Meinel]

That specific problem (struct A returning concrete struct B objects),
definitely came up in our struct vs interface discussions. It might be
better to have it as a free function rather than a method on Machine. Like a
func UnitsForMachine (machine interface {Tag ()}) [] Unit

The problem with interfaces at this level, though, is that it is one of
those "anyone that needs some attribute of Unit might use this function".

So I would tend to say, we arent set up to mock at this level. Instead
write your new code to take a factory func rather than assuming it is a
method of the types passed in. (interface definitions are intentionally not
recursive, but the factory shim should be small)

John
=:->
On Nov 1, 2013 5:42 AM, "Andrew Wilkins" <andrew.wilkins at canonical.com>
wrote:

> On Thu, Oct 31, 2013 at 8:57 PM, John Arbash Meinel <
> john at arbash-meinel.com> wrote:
>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On 2013-10-31 16:21, Andrew Wilkins wrote:
>> > Does anybody object to converting the public entity structs in
>> > state to interfaces? i.e. State, Machine, Unit, etc.
>> >
>> > I'd like to write some tests for new code that depends on state.
>> > Setting up mongo and so on is overkill. and should be relegated to
>> > integration testing.
>> >
>> > Cheers, Andrew
>> >
>> >
>>
>> I would say that if you want an interface you probably don't need the
>> whole set of functions that Unit/Machine/etc provide. And a smaller
>> interface means you don't have to create a fake one that has a bunch
>> of methods you don't actually use.
>>
>
> Agreed. I would prefer not to convert them to interfaces at all, and have
> only interfaces at the usage site. However, the way the structs are
> interrelated is problematic. In my specific use case, I really only care
> about the following hypothetical interfaces. They're being used to clean up
> an environment (destroy all units, wait for them to disappear, then destroy
> all machines without JobHostEnviron and wait for them to disappear).
>
> ----------------- 8< -------------------
>
> type State interface {
>     AllMachines() ([]Machine, error)
> }
>
> // another name would be needed, as this clashes with state.Entity; just
> for demonstration
> type Entity interface {
>     Tag() string
>     Destroy() error
>     Refresh() error
> }
>
> type Machine interface {
>     Entity
>     Units() ([]Unit, error)
>     Jobs() []MachineJob
> }
>
> type Unit interface {
>     Entity
>     IsPrincipal() bool
> }
>
> ----------------- 8< -------------------
>
> If it weren't for State.AllMachines and Machine.Units, I wouldn't have
> even asked the question. The problem is that these methods' signatures
> would need to change to return interfaces. Then once you do that, you're
> destined to implement everything as an interface. For the structs to
> implement an external interface, the structs (as they are now) would need
> to be wrapped, and then you're back to duplicating everything again.
>
> Another option would be to add a couple of new methods:
> State.AllMachineNames(), and Machine.UnitNames(), which return []strings.
> This isn't great either, as it means duplicating work (at least two trips
> to the database, when one would do).
>
> Thanks for the feedback. Creating huge interfaces didn't sound great to me
> either, but I'm not sure what a good alternative is here. I'll have to
> think some more about it... if anyone has a reasonable solution, please let
> me know.
>
> Something to think about, at least.
>>
>> I personally like having interfaces, but I'm not sure that all of
>> Unit/Machine/etc need to be turned into an interface. There are
>> already a few that we have (like Entity, etc).
>>
>> John
>> =:->
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1.4.13 (Cygwin)
>> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>>
>> iEYEARECAAYFAlJyU64ACgkQJdeBCYSNAAMJPQCg2D+tL2gx2+U4cYFBsnf9/uQW
>> ofsAnjnS8bgxEd560bI0N80ilFdFFaxK
>> =stRr
>> -----END PGP SIGNATURE-----
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20131101/c7378963/attachment-0001.html>