SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.92k stars 1.23k forks source link

ODataParentBinding: Return this to allow easy chaining #4020

Closed moreRGB closed 3 months ago

moreRGB commented 3 months ago

Hi,

we use internally a combination of suspend, filter, sort and resume to update a list binding.

filter and sort return this to allow something like this:

listbinding.sort(..).filter(..);

However suspend and resume do not return this.

This PR fixes this.

Thank you very much

cla-assistant[bot] commented 3 months ago

CLA assistant check
All committers have signed the CLA.

viktorsperling commented 3 months ago

Hello @moreRGB,

Thank you for your pull request. I've created an internal record DINC0113785. The status of the issue will be updated here in GitHub.

Regards, Viktor

ThomasChadzelek commented 3 months ago

What exactly is the use case for method chaining after resume?

ThomasChadzelek commented 3 months ago

Hello @moreRGB !

I understand that you would like to write s.th. like listbinding.suspend().sort(...).filter(...).resume(). Note that you could not include .changeParameters({$search : ...) because it also does not support this kind of method chaining. Actually, it looks like sort and filter are the two exceptions to the rule.

I am not convinced that s.th. as valuable as a method's return value should be wasted for s.th. as mundane as this. suspend might need to tell whether it actually did s.th., resume might want to return a promise for timing etc.

From another perspective, more generic code anyway first needs to check if the list binding's root binding is already suspended, suspend that root binding instead of the list binding itself, and later resume only if suspend was called before. Chaining does not help such code.

Also, v4.ODataModel alone should not do such a change which is not in line with its base classes. Especially given the strong trend towards TypeScript these days!

Overall, as the team architect of UI5's v4.ODataModel, I am not in favour of this approach. Sorry for any inconvenience caused. Thanks for using OData V4 and contributing!

Best regards, Thomas