Code review comment for lp:~oif-team/geis/geis2_add_regions

On Tue, 2010-12-21 at 18:28 +0000, Chase Douglas wrote:
> I know this has been merged, but I would like to see geis_region_delete() renamed to something
> like geis_region_deref() or geis_region_release(). Delete implies that the object will be freed
> immediately, which is not what happens here if a reference is held elsewhere.

That would be awfully confusing for the library user, considering there
is no geis_region_ref() anywhere in sight. In fact, the library user
has no idea this is a reference counted object. As far as they are
concerned, when they call geis_region_delete(), the object is deleted
and is no longer valid.

The refcounting is an internal feature of this particular object to
ensure persistence event though lifetime control is in the user's hands.
The refcounting is explicit on other objects because they are created by
the API rather than the user.

The rest of the library uses _new() and _delete() as the construction
and destruction calls where object lifetime is under the control of the
user. Consistency in an API is a good thing.

I'm less worried about what has to go on under the hood to make things
easier for the library user, since I'm assuming anyone who is
maintaining the library is capable of figuring things out by reading the
code. The typical library user should not have to read the code.

--
Stephen M. Webb <email address hidden>
Canonical Ltd.