Re: [Monotone-devel] automate show_conflict

[Zack Weinberg]

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;
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 :-(

zw