Please, no more types called "State"

[roger peppe]

On 12 March 2015 at 07:22, Andrew Wilkins <andrew.wilkins at canonical.com> wrote:
> On Thu, Mar 12, 2015 at 2:53 PM, Tim Penhey <tim.penhey at canonical.com>
> wrote:
>>
>> On 12/03/15 18:13, Ian Booth wrote:
>> > I see the point. But it could be considered analogous to having lots of
>> > methods
>> > called New() etc. So long as the types are relevant for the package in
>> > which
>> > they're declared then isn't that ok? If we have lots of packages where
>> > state
>> > needs to be persisted, how is that different to having lots of packages
>> > where a
>> > struct needs to be created, hence there will be several different New()
>> > methods.
>> >
>> > Many of the current usages are client facades in the various API
>> > packages, which
>> > is indeed unfortunate and I wish were different. But let's not
>> > universally
>> > reject State types without considering the intended semantics.
>>
>> *cough* *bullshit* *cough*
>>
>> State is a terrible name for a structure.
>
>
> So, I'm sure you know this, but for reference: I am the perpetrator of the
> latest State.
>
> Context is important to a name, and in this case I think it's obvious enough
> from the
> context what the type name means. I even made the type unexported, so we
> wouldn't
> have to have this discussion. (The grep shows it up because it's exported to
> tests in
> same package.)
>
>> I've also heard you say as much before too.
>>
>> I think people have just gotten lazy, and rather than thinking of a more
>> appropriate name, just use State because others have.  I know I'm guilty
>> of doing this.
>
>
> I object to the implication that I did this out of laziness. There's two
> reasons why
> I went ahead with the name "state":
>  1. I *did* think about this, but couldn't think of a better name. When I
> asked for
>    suggestions, I was met with silence. It's fine to say something's bad,
> but please
>    explain why and suggest alternatives.
>  2. Other have used the name :)
>
> To expand on the second point: this is not just "he did it, I'll just copy
> him and
> stop thinking now", it's "we have a precedent here, let's converge on a
> single
> vocabulary". The uniter/storage package, where this state type exists, is
> *very*
> similar to the uniter/relation code. If we're going to invent new names, we
> should
> at least be consistent about it. These bits of code are state machines;
> state machines
> require state.

For those types, I wonder if a "LocalState" name convention might
work OK - so we'd know that these types are about maintaining
the local state of the uniter rather than communicating with the
global juju state. That said, the doc comments are clear and
I doubt you're going to be confused for long by that code.

  cheers,
    rog.