[opensource-dev] Review Request: Storm-1128 Sort the results of using search in the World Map

Boroondas Gupte sllists at boroon.dasgupta.ch
Sat Apr 16 10:00:09 PDT 2011



> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110416/90867e6a/attachment.htm 


More information about the opensource-dev mailing list