Re: [Monotone-devel] Re: RFC: workspace migration and work.cc refactor [net.venge.monotone.workspace-merge.migration]

[Zack Weinberg]

On 7/31/06, Graydon Hoare <address@hidden> wrote
> Zack Weinberg wrote:
> > However, what it does have is references to the database and
> > lua_hooks objects.
>
> Ownership as sub-objects, or pointers, or C++ & references?

Presently, &-references, like this:

struct workspace {
 ...
 workspace(database & db, lua_hooks & lua) : db(db), lua(lua) {}
private:
 database & db;
 lua_hooks & lua;
};

struct app_state {
  ...
  database db;
  lua_hooks lua;
  workspace work;
  ...
};

app_state::app_state() : ... work(db, lua), ... { ... }

If I switch to holding a reference to the entire app_state object (
which I am currently leaning in favor of ) I might have to change it
to work the way database does it, i.e. with an * pointer and an
uglified name (__app, not app).  I'd prefer to keep it a reference if
possible.

As a workspace object is only ever constructed as a member of an
app_state object, the references it gets then are to the containing
object's database and lua_hooks, and nothing ever changes them
afterward, there are no lifetime issues; nor would there be if I made
it a reference to the entire app_state.

> What I'd like are possibly contradictory goals:
>
>    - minimize the use of raw pointers
>
>    - keep everything in a tree under a single root object
>
>    - move sensible subsets of app_state into sub-objects (options,
>      workspace state, etc.) to improve program clarity
>
>    - don't introduce so much "information hiding" that parts of the
>      program do unusual amounts of work to get the information they
>      need anyways
>
> If you're doing all that, or approximating it tastefully, I'm all in
> favor of refactoring. I haven't read your branch yet though.

I *think* that what I've done advances us on all the above fronts
(except raw pointers, where I didn't introduce any new ones but didn't
get rid of any old ones either); my primary concern with the way it is
now is that a small minority of app.work.foo() calls take an app_state
reference and there's no rhyme nor reason to which of them it is; thus
the thought that it might be better to put a reference to the entire
app_state into the workspace object.  [It would not help to add more
sub-component references, because the primary remaining use of
app_state is to construct node_restriction objects.]

zw