1. Unused require statement

Line4 could be deleted.

2. Unused code _dateBound = true

Is this _dateBound really still needed? Is it previously introduced for lifecycle issue solution?

Comment by Jerry on 2015-01-23 17:00PM OK I am wrong.

3. Robustness of sap.cus.crm.lib.reuse.controls.Note.prototype.setModel

It is better not to make any assumptions that the consumer will call this method setModel as we expected.
Is there any possibility that all the internal reference like _noteTypeItemTemplate is still not initialized.
In this case, the method execution will cause javascript error.

4. This.getModel() VS model

Why not use model.getProperty directly? I have verified in debugger, there are exactly the same reference?

5. Naming convention

We can not judge the real content contained in these two variables at a first glance – A little confused about the meaning of noteType and _noteType.

Is it possible to add more information inline to variable _noteType, for example change _noteType
To _oNoteTypeSelect, so that the one who reads the source code could immediately know it is a reference for select control?

6. Not necessary to loop the whole data set every time to get language description

It is possible to have an inner buffer and every time we first try if the corresponding language description is already in inner buffer already:

If ( oLanguageDescriptionSet[languageCode] ) {return oLanguageDescriptionSet[languageCode];
}
else {// old logic
oLanguageDescriptionSet[languageCode] = <fetched description via loop>
}

Another example is setAggregation implementation by ui5 framework:
Get:

Set:

This logic is widely used in ui5 framework:

7. Unnecessary variable isDefault

Previously I assume there is some logic on isDefault like if ( XXX ) { isDefault = true } else { isDefault = false }

But actually it is not. Why not directly pass a true in line 51?

8. Define constant

Can we define some “constant” in init method to avoid resourceBundle call repeatedly?
For example, in init method,
Var EDIT_NOTE_DIALOG_TITLE = this.oResourceBundle.getText(“EDIT_NOTE_DIALOG_TITLE”);

9. Unnecessary variable

Use return new sap.ui.model.json.JSONModel(oData) instead. Comment by Jerry on 2015-01-23 17:15PM
Not necessary to change: 因为发现UI5的框架代码也这样用的：

10. Nested if

A little bit ugly, can we use the following one?


If( changeType === “” ) && ( !this._hasNote(noteType, code)) {Languages.push(instance);
}

11. Is jQuery.proxy really necessary?

Haven’t yet measured the overhead of jQuery.proxy.

Some open source framework just use the following approach to avoid the additional jQuery.proxy call:

Also our own ui5 framework implementation:
doSo

12. Magic number

Can we use “constant” to avoid this magic number?

13. Better variable name?

After I go through the whole source code, I get to know that for note creation and edit case, we use the same
dialog instance, right? So _noteCreateDialog could be used both for create and Edit case?
In that case, it is better to rename _noteCreateDialog as _noteOperationDialog?

In the code below, it may confuse reader that the _getDialog can only construct Dialog for creation purpose.

14. Inconsistent naming convention

Sometimes we use prefix o to indicate the variable has object type, sometimes not, e.g noteCreateDialog.

15. Better method name

The below method assembles and finally return a model for given purpose – note creation and update.

Normally, we have two styles below:

Void doSomething() {  // do something
.. } <return type> getSomething()  { // prepare something Return something;
}

So better name like getCalculatedModel4AddNoteDialog.
Comment by Jerry on 2015-01-23 17:14PM
Just see a similar usage as ours in JSONModel,js …. , so not necessary to change?

要获取更多Jerry的原创文章，请关注公众号"汪子熙":

SAP UI5 CRM Reuse Fiori应用 note.js代码审查结果相关推荐

关于SAP UI5 CRM Reuse Fiori应用的代码审查
目录 1. Question about maven build output If we open the target folder: What is exactly the usage of t ...
SAP UI5 的本地 Fiori sandbox Launchpad
源代码如下: <!DOCTYPE html> <html><head><meta http-equiv="X-UA-Compatible" ...
SAP UI5 初学者教程之五：视图控制器初探试读版
Jerry 从 2014 年加入 SAP成都研究院 CRM Fiori 开发团队之后开始接触 SAP UI5,曾经在 SAP 社区和"汪子熙"微信公众号上发表过多篇关于 SAP U ...
SAP UI5 OData谣言粉碎机：极短时间内发送两个Odata request,前一个会自动被cancel掉吗
这是Jerry 2021年的第 9 篇文章,也是汪子熙公众号总共第 280 篇原创文章. 本文Jerry原本写于2016年5月,当时发布于团队内部wiki. 五年过后,Jerry使用的开发框架,从SA ...
SAP UI5 应用开发教程之一百零三 - 如何在 SAP UI5 应用中消费第三方库试读版
一套适合 SAP UI5 初学者循序渐进的学习教程作者简介 Jerry Wang,2007 年从电子科技大学计算机专业硕士毕业后加入 SAP 成都研究院工作至今.Jerry 是 SAP 社区导师,S ...
如何在 SAP UI5 应用中集成第三方库：一个在移动设备上查看 Web 应用打印调试信息的小技巧
这是 Jerry 2021 年的第 43 篇文章,也是汪子熙公众号总共第 320 篇原创文章. 做 Web 开发的程序员,无论使用 SAP UI5,还是 Angular,React,Vue,每天都离不 ...
SAP UI5 应用 index.html 里 data-sap-ui-resourceroots 指令的含义和作用
如下图所示: <script id="sap-ui-bootstrap"src="https://sapui5.hana.ondemand.com/resource ...
SAP UI5 应用开发教程之五十五 - 如何将本地 SAP UI5 应用通过 Node.js Express 部署到公网上试读版
一套适合 SAP UI5 初学者循序渐进的学习教程教程目录 SAP UI5 本地开发环境的搭建 SAP UI5 应用开发教程之一:Hello World SAP UI5 应用开发教程之二:SAP U ...
SAP UI5 应用开发教程之五十四 - 如何将本地 SAP UI5 应用配置到本地 Fiori Launchpad 中的试读版
一套适合 SAP UI5 初学者循序渐进的学习教程教程目录 SAP UI5 本地开发环境的搭建 SAP UI5 应用开发教程之一:Hello World SAP UI5 应用开发教程之二:SAP U ...

SAP UI5 CRM Reuse Fiori应用 note.js代码审查结果

目录