Skip to content

Use Promise for Commands#95

Merged
gchoqueux merged 2 commits intoiTowns:masterfrom
peppsac:command_with_promise
Jul 1, 2016
Merged

Use Promise for Commands#95
gchoqueux merged 2 commits intoiTowns:masterfrom
peppsac:command_with_promise

Conversation

@peppsac
Copy link
Contributor

@peppsac peppsac commented Jun 30, 2016

Commands are async, so Promise are a good fit for this usage.

It will ease the transition to remove logic code from Providers.
Simplified example:

    // calling site
    interCommand.request(args, node);

    // provider
    Provider.prototype.executeCommand = function(command){
        return this.getColorTextures(tile, services).then(function(result)
        {
            this.setTexturesLayer(result, destination);
        }.bind(tile));

Would become:

    // calling site
    interCommand.request(args, node).then(function(textures) {
          node.setTexturesLayer(result,destination);
    });

    // provider
    Provider.prototype.executeCommand = function(command){
        return this.getColorTextures(tile, services).then(function(result)
        {
            command.resolve(result);
        };

peppsac added 2 commits June 30, 2016 18:02
Commands are async, so Promise are a good fit for this usage.

It will ease the transition to remove logic code from Providers.
Simplified example:

    // calling site
    interCommand.request(args, node);

    // provider
    Provider.prototype.executeCommand = function(command){
        return this.getColorTextures(tile, services).then(function(result)
        {
            this.setTexturesLayer(result, destination);
        }.bind(tile));

Would become:

    // calling site
    interCommand.request(args, node).then(function(textures) {
          node.setTexturesLayer(result,destination);
    });

    // provider
    Provider.prototype.executeCommand = function(command){
        return this.getColorTextures(tile, services).then(function(result)
        {
            command.resolve(result);
        };
@peppsac
Copy link
Contributor Author

peppsac commented Jun 30, 2016

I added a 2nd commit that modifies WMTS_Provider and TileProvider to take advantages of these Promise.

@qdnguyen
Copy link
Contributor

qdnguyen commented Jul 1, 2016

Cool but if you can add web workers in this module to parallelize tasks, it is even better!

@peppsac
Copy link
Contributor Author

peppsac commented Jul 1, 2016

Cool but if you can add web workers in this module to parallelize tasks, it is even better!

For now this PR is focused on getting the groundwork done so we can improve the code later -and yes Web Workers would be nice to have :-)

gchoqueux added a commit that referenced this pull request Jul 1, 2016
@gchoqueux gchoqueux merged commit 2954875 into iTowns:master Jul 1, 2016
gchoqueux added a commit that referenced this pull request Jul 1, 2016
#62 add api functions : removeImageryLayer, moveLayerToIndex
@gcebelieu
Copy link

Be careful, promise is ES6 and doesn't seem to be supported by IE11 (see : http://caniuse.com/#feat=promises).
To add this support, a promise polyfill (https://github.com/lahmatiy/es6-promise-polyfill) should be added.

@tbroyer
Copy link
Contributor

tbroyer commented Jul 4, 2016

@gcebelieu This is using when promises, not native promises. But IMO eventually promises should be moved to native ones with a polyfill.

BTW, https://github.com/stefanpenner/es6-promise is better maintained and seems to be the go-to polyfill.

@peppsac
Copy link
Contributor Author

peppsac commented Jul 4, 2016

See #97

@gcebelieu
Copy link

thanks, this sounds good to me :)

NikoSaul pushed a commit to NikoSaul/itowns2 that referenced this pull request Jul 4, 2017
NikoSaul pushed a commit to NikoSaul/itowns2 that referenced this pull request Jul 4, 2017
iTowns#62 add api functions : removeImageryLayer, moveLayerToIndex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants