On Mon, Jul 21, 2003 at 10:42:53AM +0000, Angus Leeming spake thusly: > > Martin Vermeer wrote: > > Does this look ready to go in? ... > You mean 'Assert(false)' here I think.
Yes... assert always.
> +string BranchList::getColor(string const & s) const
> ...
> + Assert(true);
> + return string();
> +}
>
> Can 's' belong to multiple branches?
No.
> If not you should have a 'break' in
> there. Your Assert isn't doing much at the moment ;-)
It should catch the 'branch not existing' condition and return an
empty string. I believe Assert(false) will do that.
> +void BranchList::setColor(string const & s, string const & val)
> +{
> + List::iterator it = list.begin();
> + List::iterator end = list.end();
> + for (; it != end; it++) {
> + if (s.find(it->getBranch(), 0) != string::npos) {
> + it->setColor(val);
> break;
> + }
> + }
> + Assert(true);
> Assert(false);
> +}
Now *I* don't understand.
...
> I think the logic here is wrong.
> +void BranchList::add(string const & s)
> +{
> + string name;
> + size_t i = 0;
> + size_t j = 0;
> + Branch br;
> + while (j < string::npos) {
> + j = s.find_first_of(separator(), i);
> + name = s.substr(i, j - i);
> + br.setBranch(name);
> + br.setSelected(false);
> + br.setBranch(name);
> + br.setColor("none");
> + list.push_back(br);
> + i = j;
> + i++;
> + }
> +}
>
> for (;;) {
> string::size_type const j = s.find_first_of(separator(), i);
> if (j == string::npos)
> break;
> ...
> }
>
> Ditto for setColors.
I don't think it's wrong, but yours looks better. Compiling now.
> I'm not an author of this file, or of FormBranch.h, or indeed of
> insetbranch.[Ch] ;-)
I boilerplated from your code. SCO would *insist* on proper
consideration :-(
...
> You could reasonably add yourself as an author of FormDocument.C
>
> You've added some spurious white space to insetnote.h (although you have
> also removed some from insetnote.C).
Oops...
> --
> Angus
>
Martin
pgp00000.pgp
Description: PGP signature
