> On April 13, 2011, 2:31 p.m., Boroondas Gupte wrote: > > indra/newview/llfloaterworldmap.cpp, lines 99-101 > > <http://codereview.secondlife.com/r/262/diff/1/?file=1462#file1462line99> > > > > Why do you make this a functor rather than a plain normal function? > > It's not like we have to keep any internal state or something. > > Jonathan Yap wrote: > Moved to inside of routine. > > Boroondas Gupte wrote: > That's a good idea, too, but not what I meant. I was wondering why you > are using a class (or struct) with an operator() instead of just a function. > std::sort can take either a functor (i.e. a callable object like you're using > here) or a function pointer. > > Jonathan Yap wrote: > Nothing else in this class would make use of this function so it seemed > to make sense to go the struct route (plus when I was trying to figure out > how to code this initially the example I found elsewhere in the code was done > this way). > > Boroondas Gupte wrote: > Ah, right. I forgot local function aren't allowed in C++. Otherwise one > could have done something like > inline bool sortRegionNames(std::pair <U64, LLSimInfo*>& _left, > std::pair <U64, LLSimInfo*>& _right) > { > > return(LLStringUtil::compareInsensitive(_left.second->getName(),_right.second->getName()) > < 0); > } > > std::vector<std::pair <U64, LLSimInfo*> > > sim_info_vec(LLWorldMap::getInstance()->getRegionMap().begin(), > LLWorldMap::getInstance()->getRegionMap().end()); > std::sort(sim_info_vec.begin(), sim_info_vec.end(), &sortRegionNames); > > So we probably have to either stick to your struct or make > sortRegionNames a method or free function. > > Jonathan Yap wrote: > After fixing the >> to > > issues are you able to compile if the struct I > moved from the top of the code into this routine is moved back to the top > again? The other example I found had it that way, and now I am wondering if > it was like that to allow linux to compile.
With storm-1128c.diff (r3 of this review request) the viewer compiles fine, but only because it also changes the arguments of the comparison function to const references. (We discussed this on IRC.) See also http://stackoverflow.com/questions/4084035/stl-sort-issue-with-gcc I still got some comments about the code, will post them in a review of the r3 diff. - Boroondas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/262/#review595 ----------------------------------------------------------- On April 16, 2011, 6:24 a.m., Jonathan Yap wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/262/ > ----------------------------------------------------------- > > (Updated April 16, 2011, 6:24 a.m.) > > > Review request for Viewer. > > > Summary > ------- > > The results of using the World Map search option are sorted. > > > This addresses bug Storm-1128. > http://jira.secondlife.com/browse/Storm-1128 > > > Diffs > ----- > > doc/contributions.txt a8f868007986 > indra/newview/llfloaterworldmap.cpp a8f868007986 > > Diff: http://codereview.secondlife.com/r/262/diff > > > Testing > ------- > > > Thanks, > > Jonathan > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
