On 23/08/13 21:37, roger peppe wrote: > On 23 August 2013 04:11, Tim Penhey <tim.pen...@canonical.com> wrote: >> // TODO (thumper): 2013-08-23 bug 1654321 >> // Details about the change needed, or things to do. > > This seems reasonable, but there's one down side - currently > for most TODOs, a grep will pull out both the TODO and its > description, whereas this will lose that. > > Perhaps we could do it this other way around? > > // TODO (thumper) Details about the change needed > // 2013-08-23 bug 1654321
It depends on what you think is the more important information. Also, as William mentions, there are many places where the details are multi-line. It makes sense to have the defined structure on the first line, and the rest is the details. >> Secondly, I propose the following guideline: >> >> Prefer exporting functions and interfaces to structures >> - unless there is a very good reason why the structure >> should be exported. >> >> A common example of a structure that should be exported is a "params" >> type structure that groups parameters for another exported function. >> >> By exporting interfaces, we have more ability to mock the behaviour for >> testing, and to change implementation details without impacting all the >> users of the code. > > I don't quite get this. There's nothing about *returning* an interface > that makes more easily mockable, AFAICS. The thing that makes things > easier to mock is that *parameters* are interfaces, as narrow as > possible for the code in question. Doing things this way makes it > the responsibility of each piece of code to enable itself to be mocked, > while allowing it to present the most natural API possible to its > clients. OK, there are several key problems we are trying to solve, and the general policy specified above is trying to address the problems, not be some religious, carved-in-stone, hard rule. A concrete example that I've been working on is the agent config. The exposed type, with all public data members, tied the implementation details to the use of the object. Getting the agent config to be provided by an interface, so we can change how the config is stored was an epic task because of the creep of public data poking. If you expose a structure with all public data, and have methods that rely on the relationship between data members, then you have a problem (as we did with agent config). I'm not saying that exporting structures is always bad. I'm saying that it is often bad. As William's example demonstrated, we don't want pointers to structures to be returned from interface methods (unless the structures are "plain old data" type structures). If you have exported structures with private members, you can't easily generate them from other packages. A key issue here is data hiding. Go gives us a great facility to do this using interfaces. > For example, in the Go library, bufio.NewReader returns a concrete type, > not an interface, but many methods and functions will accept an > io.Reader interface > value, which happens to be implemented by bufio.Reader. > This is idiomatic Go, and I think it can work well. Yes this can be fine, but for very small, well defined types. A lot of juju-core doesn't fit this model. Let's not try to say "oh, this is idiomatic Go so we should always do this in juju-core", because that way lies a world of hurt. There are some design patterns for large scale software that is generally used no matter what language you are writing it. > Moreover some types really don't need to be mocked, as > they're easily created on the fly and their methods don't > rely on anything other than the data contained within them. > Would there be any advantage in, for example, > passing constraints.Value around as an interface? As I said above, the policy is "prefer". Since constraints.Value is more of a value type, and public only data, it is easily constructable and not tightly coupled to other things. >> This is considered good programming practice. > > I'm concerned that this statement is much more true > in a Java-centric world than a Go-centric one. Information hiding and abstracting implementation details from use is pretty general. Tim -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
