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

Add sort for ListView #107

Merged
merged 3 commits into from Sep 7, 2019

Conversation

@Kvaz1r
Copy link
Contributor

Kvaz1r commented Sep 7, 2019

I not think that it's the best solution but as for me it's still better than removing all items and adding them back on each sorting.

If these changes is appropriate I'll also add tests.

@texus

This comment has been minimized.

Copy link
Owner

texus commented Sep 7, 2019

This definitely looks like a useful function.

Could you perhaps swap the parameters of the sort function (so first the index and then the comparator)? I think that it might be slightly more readably when calling the function with a lambda function as comparator.

I don't really like that both variables are initialized on one line, so maybe you could also either use a ternary operator (and make s1 and s2 const) or rewrite the function as follows?

sf::String s1;
if (index < a.texts.size())
    s1 = a.texts[index].getString();

sf::String s2;
if (index < b.texts.size())
    s2 = b.texts[index].getString();
@Kvaz1r

This comment has been minimized.

Copy link
Contributor Author

Kvaz1r commented Sep 7, 2019

good points, I'll fix it.

Kvaz1r added 2 commits Sep 7, 2019
@texus texus merged commit 95b7002 into texus:0.8 Sep 7, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Kvaz1r Kvaz1r deleted the Kvaz1r:SortListView branch Sep 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.