[Top][All Lists]

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

Re: [Monotone-devel] automate show_conflict

From: Zack Weinberg
Subject: Re: [Monotone-devel] automate show_conflict
Date: Thu, 17 Apr 2008 19:08:50 -0400

No comment on anything else - I will try to find time to read the
patch in the next few days - but

On Thu, Apr 17, 2008 at 6:22 PM, Thomas Keller <address@hidden> wrote:
>  +  boost::shared_ptr<roster_t const> 
> left_roster(db_adaptor.rosters[db_adaptor.left_rid]);
>  +  I(0 != left_roster);
>  +  boost::shared_ptr<roster_t const> 
> right_roster(db_adaptor.rosters[db_adaptor.right_rid]);
>  +  I(0 != right_roster);
>  I don't know much of boost::shared_ptr, but basically any of both
> invariants would fire if the db_adaptor.rosters map does not have the
> revision roster cached, correct? If we need invariants there, wouldn't it be
> better to check the root of the paranoia, i.e.
>  I(db_adaptor.rosters.find(db_adaptor.left_rid) != db_adaptor.rosters.end())

... please don't do things that will make it do tree searches twice in
a row in the common case.  I'd probably write this as

typedef map<revision_id, shared_ptr<roster_t const> >::iterator
rid_roster_iterator left_roster_p = db_adaptor.rosters.find(left_rid);
I(left_roster_p != db_adaptor.rosters.end());
// ... etc ...

but even that looks rather ugly :-(


reply via email to

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