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

Replacing the _getIndex method by UnderscoreJS method _.findIndex() #2157

Merged
merged 5 commits into from May 17, 2016

Conversation

Projects
None yet
5 participants
@anuprulez
Copy link
Member

commented Apr 13, 2016

  • Reasons

The current 'getIndex' method returns the index of the specified value if present in the list/ array. This method can be replaced by one line code (a method of Underscore JS method) '_.findIndex' (UnderscoreJS methods are compactly written and faster). This method also takes care of the case when the queried value is not found in the list (returns -1 if value is not found. So no need to write one extra line for returning -1).

  • Changes

Previous

        // search index
        for (var key in this.select_data) {
            if (this.select_data[key].id == value) {
                return key;
            }
        }
        // not found
        return -1;

Committed:

_.findIndex(this.select_data, {id: value});
  • Tests

The test cases would remain the same as the values returned by the method remains the same.


// Old code
//for (var key in this.select_data) {

This comment has been minimized.

Copy link
@bgruening

bgruening Apr 13, 2016

Member

The old code is not needed. Moreover you need to run make client from your GALAXY_ROOT to build the js libs.
Thanks @anuprulez !

@galaxybot galaxybot added the triage label Apr 13, 2016

@galaxybot galaxybot added this to the 16.07 milestone Apr 13, 2016

@anuprulez

This comment has been minimized.

Copy link
Member Author

commented Apr 13, 2016

For this commit, I have used an Underscore JS method '_findIndex' which is available in version 1.8.0
http://underscorejs.org/#findIndex but in the Galaxy (https://github.com/galaxyproject/galaxy/blob/dev/client/galaxy/scripts/libs/underscore.js) we are using 1.7.0. So the build is failing as that is method is not defined in the older version.

Work In Progress. Please do not merge.

@anuprulez

This comment has been minimized.

Copy link
Member Author

commented Apr 13, 2016

Work In Progress. Please do not merge.

@bgruening bgruening changed the title Replacing the _getIndex method by UnderscoreJS method '_.findIndex' WIP: Replacing the _getIndex method by UnderscoreJS method _.findIndex() Apr 13, 2016

@bgruening

This comment has been minimized.

Copy link
Member

commented May 12, 2016

@anuprulez please have a look at this one: #2328

@anuprulez

This comment has been minimized.

Copy link
Member Author

commented May 12, 2016

@bgruening: I have committed a change in bower.json file for downloading the latest version (1.8.3) of UnderscoreJS.

@anuprulez anuprulez changed the title WIP: Replacing the _getIndex method by UnderscoreJS method _.findIndex() Replacing the _getIndex method by UnderscoreJS method _.findIndex() May 12, 2016

@bgruening bgruening removed the status/WIP label May 13, 2016

@bgruening

This comment has been minimized.

Copy link
Member

commented May 13, 2016

@carlfeberhard /@guerler does this looks good?

@martenson martenson merged commit 4d6b8bf into galaxyproject:dev May 17, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@martenson

This comment has been minimized.

Copy link
Member

commented May 17, 2016

merged manually, thanks for the contribution @anuprulez !

@bgruening

This comment has been minimized.

Copy link
Member

commented May 17, 2016

Awesome! Thanks @anuprulez @martenson!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.