On Fri, 2004-07-16 at 15:12, Aubanel MONNIER wrote:
A std::vector<BBoxSort *> in fact, then using v.sort; FOr the momment, I used a std::vector<BBoxSort > instead, the copy contructor not being too heavy. But I would prefer using pointers, for sure ...
Yes. Definitely a place for pointers.
In fact, I'm not used to smart pointers, but to answering one of my questions, I was suggested to do it that way.
It could be one of the less evil uses of auto pointers, though there are still probably better approaches. I would need to know more about the code in question before I could know what to suggest, however.
The downside to smart pointers here would be that (with the extra indirection, depending on cache effects) they could cost as much as or more than the copy constructor.
SPSelection *selection = SP_DT_SELECTION(desktop); if (!selection) return; std::list<SPItem *> selected; selection->list(selected); if (selected.empty()) return;
This I'm a little less comfortable with. Rather than copying the list I think we should just provide a method that returns a Glib::Sequence. It wouldn't limit the client to std::lists, either.
BBoxSort b (*it, _orientation, _kBegin, _kEnd); sorted.push_back(b);
Note that the copy constructors potentially also cost you here each time the vector is resized, as the entire vector is copied to new memory.
} //sort bbox by anchors std::sort(sorted.begin(), sorted.end());/*[Snipped code using the sorted vector] */
}
Using the garbage collector would probably be easiest here; all you need to do is make BBoxSort a subclass of one of the "collectable" base classes I'll be introducing.
BBoxSort(const BBoxSort &rhs): //NOTE : this copy ctor is called O(sort) when sorting the vector //this is bad. The vector should be a vector of pointers. //But I'll wait the bohem GC before doing that item(rhs.item), anchor(rhs.anchor), bbox(rhs.bbox) { }
I learned recently that writing the copy constructor here isn't actually necessary -- if you don't explicitly provide one the C++ compiler will generate one for you.
(This is also true of assignment operators)
A corrolary to this is that if you really don't want a class to have a copy constructor or assignment operator, you will have to settle for making the declaration private and leaving out the definition.
( C++ is fun, isn't it? I've been doing this for many years, and I still learn about a new nuance of the language every week. :P )
Anyway, what I'd probably suggest would be something like (renaming according to our coding standards -- I'm assuming the method is private or protected):
struct BBoxSort : public Inkscape::GC::Managed<> { /* etc.. */ };
virtual void _onButtonClick() { SPDesktop *desktop=SP_ACTIVE_DESKTOP; if (!desktop) return;
SPSelection *selection=SP_DT_SELECTION(desktop); if (!selection) return;
Glib::Sequence<SPSelection::Iterator<SPItem *> > items(selection->items()); if ( items.begin() == items.end() ) return;
std::vector<BBoxSort *, Inkscape::GC::Alloc<> > sorted; sorted.reserve(items.size());
for ( SPSelection::Iterator<SPItem *> it(items.begin()) ; it != items.end() ; ++it ) { BBoxSort *b=new BBoxSort(*it, _orientation, _k_begin, _k_end); sorted.push_back(b); } std::sort(sorted.begin(), sorted.end());
/* use sorted vector */ }
I need to introduce the GC:: APIs and SPSelection::items() yet, of course.
-mental