lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Code review: product editor


From: Greg Chicares
Subject: Re: [lmi] Code review: product editor
Date: Sat, 24 Mar 2007 00:26:21 +0000
User-agent: Thunderbird 1.5.0.4 (Windows/20060516)

On 2007-3-21 17:36 UTC, Evgeniy Tarassov wrote:
> On 3/21/07, Greg Chicares <address@hidden> wrote:
[...]
> In DatabaseTableAdapter every time the inner tdbvalue is accessed it
> is checked to be non-NULL. I have tried to enforce it by requiring a
> non-NULL pointer but it makes the code less understandable since  a
> dummy static TDBValue has to be used and additional checks should be
> added so that this dummy variable is not modified by mistake.

Is this the reasoning?
 - Non-leaf nodes don't really have content.
 - But they present a GUI for modifying content, so they have
     to be given dummy content.
 - But that dummy content shouldn't be modified, so we add
     code to prevent modification.
 - And that requires extra work...which is easier if dummy
     content is NULL?

What if, instead, we presented no GUI for modifying the
(nonexistent or meaningless) content of non-leaf nodes? or,
equivalently from the user's POV, left that GUI in place but
made it invisible when a non-leaf node is selected (if that's
easier)? Then we wouldn't have to prevent modifying a dummy
node. And maybe dummy content wouldn't need to exist at all,
or could just be default-constructed instances.

Then we wouldn't need null pointers, would we?

>> [shared_ptr] can extend an object's lifetime; is that valuable
>> here...or could scoped_ptr have been used instead? [...]
> 
> The table_adapter_ is shared by DatabaseView and a instance of
> DatabaseEditorGrid which accepts boost::shared_ptr. We could add an
> additional field to DatabaseView which would be a reference to
> table_adapter_ but we still need to keep a shared_ptr (somewhere in a
> pocket) only to ensure it is not deleted by DatabaseEditorGrid. I'd
> add a pair of methods:
> DatabaseTableAdapter& DatabaseView::table_adapter() { *table_adapter; }
> + the const version.

Can you explain that at a higher level please? I think you're
saying there's an object that needs a longer lifetime than it
would have if it were created on the stack. I'm having trouble
seeing what that object is. Is it a database entity's multi-
dimensional data? If so, then I would think that the document
class would own it, and its lifetime would last until the
document is closed. What need for shared-ownership semantics
am I overlooking?




reply via email to

[Prev in Thread] Current Thread [Next in Thread]