Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CArrayDataProvider case insensitive #990

Closed
yannyannyann opened this issue Jul 17, 2012 · 9 comments · Fixed by #1025
Closed

CArrayDataProvider case insensitive #990

yannyannyann opened this issue Jul 17, 2012 · 9 comments · Fixed by #1025
Assignees
Milestone

Comments

@yannyannyann
Copy link

Hello

in the CArrayDataProvider class I expect the sortData() function to be case insensitive, but it is not, because it is using the php method array_multisort http://php.net/manual/en/function.array-multisort.php

I am now fixing it by creating a separate class which extends CArrayDataProvider in order to override the sortData by calling the php method strtolower() on the elements to be sorted.
(like suggested here http://www.yiiframework.com/forum/index.php/topic/17154-case-insensitive-sort/ )

Please let me know if this could be an option of the sortData in the future ?

Thanks

Yann

@marcovtwout
Copy link
Member

I would even expect case-insensitive sorting to be default.

Example nr4 on php.net suggests converting the array to lowercase values, as done in the solution above.

@bwoester
Copy link

In the end, it should be configurable. Maybe one wants to sort something containing numbers, so natural ordering would be handy. If the case insensitive algorithm is hard coded, you'll face the same problems as now: the data provider just doesn't behave the way you need it to behave.

Also, modifying the class to act case-insensitive by default is a bad idea for the 1.1.x release series, because it modifies behavior. Some applications might rely on case sensitive sorting. They shouldn't break because of updating the framework. So making it configurable, with just the same sorting as now as default, seems to be the best solution to me.

@BBoom
Copy link

BBoom commented Jul 25, 2012

Agree with bwoester, this should be configurable.

@resurtm
Copy link
Contributor

resurtm commented Jul 25, 2012

@BBoom Natural sorting addition should be in the separate issue, IMO.

@BBoom
Copy link

BBoom commented Jul 25, 2012

@resurtm Agreed, but I think making this a configurable option is a good move to allow future expanding. A first version should probably be limited to case-insensitive sorting, to speed up implementation (and also, this is the only concrete case people have reported needing). But it would be wise to implement it in such a way that other sort styles could be implemented later. In other words, not a 'caseSensitive' boolean param, but more of a 'sortingStyle' param that takes constants.

@resurtm
Copy link
Contributor

resurtm commented Jul 25, 2012

@BBoom What about another new boolean property called something like $naturalSort? Do you think this way is worse and less extendable than bit masks?

@mdomba @samdark Your opinions?

@mdomba
Copy link
Member

mdomba commented Jul 25, 2012

how about setting an expression or anonymous function?

@bwoester
Copy link

I'm not too familiar with CArrayDataProvider source, just took a quick look. From what I see, I think the current solution with case sensitive/ insensitive flag really might be enough. If someone needs more, he can always inherit and override sortData method.

However, if someone wants to do more refactoring, to make the class more flexible, I think the sortData method could be modified slightly:

Sorting direction could be removed from the signature. If descending order is configured, this can be done by CArrayDataProvider by inverting the sorted array. No need to handle sort order in every implemented algorithm.
The sort algorithm might be invoked as a callback, or object implementing an ISortable interface. CArrayDataProvider (or yii framework in general, if sort implementations are interesting in other places) might provide case sensitive/ insensitive implementations, as well as natural sorting implementations. Another sort algorithm characteristic that comes to my mind is its stability. Does the algorithm maintain element order for elements with the same comparison key?

@cebe cebe mentioned this issue Aug 1, 2012
@ghost ghost assigned samdark Sep 8, 2012
@samdark samdark closed this as completed Sep 8, 2012
@samdark
Copy link
Member

samdark commented Sep 8, 2012

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants