SAP Engagement Center Agent Home的一段代码审查
SAP Engagement Center Agent Home的一段代码审查
CEC agent Home review
Review Date: 2015-10-18
1. When I test, Ifound this json file is loaded twice with two different url. Actually the content is exactly the same. Why?
When I debug, I find it is because sandbox.js has loaded two different json files in line 278 and 288. Is this expected behavior? 2. why we should manually load this file? As far as I know, it should never be loaded by application manually.
3. why missing var before “_ref”?
4. where does this 50 minutes come from? Can it be configurable? 5. There are duplicate loads on Component-preload-dbg.js triggered by different location of home.html:
6. LaunchPadShell.controller.js global namespace pollution. When window.YaasAuthIO will be initialized?
7. unused variable
since:
8. the instance returned by getInstance() is never used. Why call this method?
can be refacted as: var logoIconURL = sLogoIcon; if (typeof sLogoIcon === “undefined” || sLogoIcon === “”) { logoIconURL = jQuery.sap.getModulePath( “sap.ui.core”, ‘/’) + “mimes/logo/sap_50x26.png”; } this.oLaunchPadUIModel = new sap.ui.model.json.JSONModel( { menuVisible : false, backVisible : false, logo : logoIconURL }); 10. why the logo picture’s size is fixed? How to display the logo in different mobile device?
12. why don’t use i18n resourceBundle to support multiple language?
13. Using this user to log on to CEC agent home, ocassionally I will meet with “insufficient permission issue”.
14. inproper error handling: Suppose there is exception occurred in line 101, so oShellHeadUserItem is undefined. Then in line 113 you set this undefined to PadShell? Why not quit process after line 109?
15. The parameter “this” is not used in com.sap.cecenter.agenthome.cctr.CctrInteraction, why pass it in?
18. question:
in Init phase, there is nothing to destroy. Why the line 45 and 49 is necessary?
19. why line 37 is necessary?
20. why don’t use for-loop to deal with array?
could be refacted as : getOneHBox: function() { return new sap.m.HBOX({ displayInline: true, layoutData: new sap.ui.layout.GridData({ span: window.screen.width >= 1440? “L2 M4 S4”: “L3 M4 S4” }) }); 21. naming convention: a. it is an object, use oTileContent instead. b. getTileContent: function(oTileContent) { return new sap.suite.ui.commons.TileContent({ footer: oTileContent.footer, unit: oTileContent.unit }); }
22. control id should not be hard coded. Refer to best practice here:
23. Why not simply use the pair of busydialog.requireBusyDialog() and releaseBusyDialog()?
correct:
25. index.html The following three lines could be combined into one line: var bCecenterIsLocal = !!jQuery.sap.getUriParameters().get(“local”);
use this solution instead: <html> <script> var href = “[https://dewdfgwd01082.wdf.sap.corp:4080/sap/opu/odata/sap/CRM_OPPORTUNITY/?q=jerry#123]{.underline}”; var match = href.match(/^(https?\?\/\/(([^:\/?#]*)(?:\?[0-9]+))?)(\/[^?#]*)(\?[^#]*|)(#.*|)$/); debugger; </script> </html>
