Code review comment for lp:~abreu-alexandre/unity-webapps-qml/application-api

Revision history for this message
Alberto Mardegan (mardy) wrote :

I think there's a problem with how the getters are implemented. Let's take getScreenOrientation() for example: when the client calls this method, it will retrieve the application name which was setup, which is correct. If the application registers a callback with onScreenOrientationChanged(), then this._screenOrientation will always get updated propertly, so getScreenOrientation() will always return the proper value; however, if the application doesn't call onScreenOrientationChanged(), this._screenOrientation will never be updated and getScreenOrientation() will always return the old value.

I'm not sure how expensive the switch betwenn client and backend is, so depending on that:

1) If the switch is not very expensive: don't cache any values in the client, always request the properties directly from the backend.

2) If the switch is very expensive, do as you are doing, but register an internal callback to the cached properties in the Application constructor. Something like:

  var self = this;
  this._name = content.name;
  this._proxy.call('onApplicationNameChanged', [function(name) {self._name = name; }]);

(In the latter case we might even want to avoid passing through the backend when the client calls onApplicationNameChanged(): we could make it store the function callback into an array (this._applicationNameListeners) and in the callback registered in the Application constructor we could then invoke all the callbacks in this._applicationNameListeners, after having updated "self._name = name")

Can you also make the getInputMethodName return the value immediately?

Other than that, it all looks good to me!

review: Needs Fixing

« Back to merge proposal