版权声明:本文为博主汪子熙原创文章,未经博主允许不得转载。 https://cloud.tencent.com/developer/article/1439616
Line4 could be deleted.
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.
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.
Why not use model.getProperty directly? I have verified in debugger, there are exactly the same reference?
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?
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:
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?
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”);
Use return new sap.ui.model.json.JSONModel(oData) instead. Comment by Jerry on 2015-01-23 17:15PM
Not necessary to change: 因为发现UI5的框架代码也这样用的:
A little bit ugly, can we use the following one?
If( changeType === “” ) && ( !this._hasNote(noteType, code)) {
Languages.push(instance);
}
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
Can we use “constant” to avoid this magic number?
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.
Sometimes we use prefix o to indicate the variable has object type, sometimes not, e.g noteCreateDialog.
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?