Martin Vermeer <[EMAIL PROTECTED]> writes:
| See attached. Against CVS of an hour ago.
I am not looking at how this work only on how it is implemented.
| +string Branch::getBranch() const
| +{
| + return branch_;
| +}
Why are you not returning a const reference?
| +string Branch::getColor() const
| +
| +{
| + return color_;
| +}
ditto
| +string BranchList::getColor(string const & s) const
| +{
| + List::const_iterator it = list.begin();
| + List::const_iterator end = list.end();
| + for (; it != end; ++it) {
| + if (s.find(it->getBranch(), 0) != string::npos) {
| + return it->getColor();
| + }
| + }
Is this check really good? what if a color is named "gray" and another
is named "grayblue"?
(hmm... color == branch?)
| +void BranchList::add(string const & s)
| +{
| + string::size_type i = 0;
| + for (;;) {
I must admit that I favour "while (true) {"
| + string name;
| + if (j == string::npos)
| + name = s.substr(i);
| + else
| + name = s.substr(i, j - i);
I wonder... wouldn't
string name = s.substr(i, j - i);
always be correct?
| +string BranchList::separator() const
| +{
| + return separator_;
| +}
return const reference?
| +<<<<<<< ChangeLog
| +2003-08-13 Martin Vermeer <[EMAIL PROTECTED]>
| +
| + * ColorHandler.[Ch]: (1) make possible a dynamically growing
| + LColor list by allowing the graphic context cache to grow along
| + (vector); (2) eliminate an IMHO unnecessary step in colour
| + allocation.
| +
| +=======
left over conflict tags. remove those
Basically I have only nit-picks, time to apply me thinks.
--
Lgb
