Skip to content

Commit 7e87221

Browse files
committed
Refactor: remove some code duplication
+ Complex statement that was executed three times moved to separate method so the logic is only on one place + The calculation for the index of the last record moved to a variable so the calculation is only being done once. This also reduces the amount of times `this.getChildItems()` is being called.
1 parent 08b8956 commit 7e87221

File tree

1 file changed

+20
-12
lines changed

1 file changed

+20
-12
lines changed

app/code/Magento/Ui/view/base/web/js/dynamic-rows/dynamic-rows.js

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -330,9 +330,7 @@ define([
330330
}
331331

332332
if (this.defaultPagesState[this.currentPage()]) {
333-
this.pagesChanged[this.currentPage()] =
334-
!compareArrays(this.defaultPagesState[this.currentPage()], this.arrayFilter(this.getChildItems()));
335-
this.changed(_.some(this.pagesChanged));
333+
this.setChangedForCurrentPage();
336334
}
337335
},
338336

@@ -442,13 +440,9 @@ define([
442440
return initialize;
443441
}));
444442

445-
this.pagesChanged[this.currentPage()] =
446-
!compareArrays(this.defaultPagesState[this.currentPage()], this.arrayFilter(this.getChildItems()));
447-
this.changed(_.some(this.pagesChanged));
443+
this.setChangedForCurrentPage();
448444
} else if (this.hasInitialPagesState[this.currentPage()]) {
449-
this.pagesChanged[this.currentPage()] =
450-
!compareArrays(this.defaultPagesState[this.currentPage()], this.arrayFilter(this.getChildItems()));
451-
this.changed(_.some(this.pagesChanged));
445+
this.setChangedForCurrentPage();
452446
}
453447
},
454448

@@ -849,7 +843,8 @@ define([
849843
deleteRecord: function (index, recordId) {
850844
var recordInstance,
851845
lastRecord,
852-
recordsData;
846+
recordsData,
847+
lastRecordIndex;
853848

854849
if (this.deleteProperty) {
855850
recordsData = this.recordData();
@@ -868,12 +863,13 @@ define([
868863
this.update = true;
869864

870865
if (~~this.currentPage() === this.pages()) {
866+
lastRecordIndex = (this.startIndex + this.getChildItems().length - 1);
871867
lastRecord =
872868
_.findWhere(this.elems(), {
873-
index: this.startIndex + this.getChildItems().length - 1
869+
index: lastRecordIndex
874870
}) ||
875871
_.findWhere(this.elems(), {
876-
index: (this.startIndex + this.getChildItems().length - 1).toString()
872+
index: lastRecordIndex.toString()
877873
});
878874

879875
lastRecord.destroy();
@@ -1134,6 +1130,18 @@ define([
11341130
});
11351131

11361132
this.isDifferedFromDefault(!_.isEqual(recordData, this.default));
1133+
},
1134+
1135+
/**
1136+
* Set the changed property if the current page is different
1137+
* than the default state
1138+
*
1139+
* @return void
1140+
*/
1141+
setChangedForCurrentPage: function () {
1142+
this.pagesChanged[this.currentPage()] =
1143+
!compareArrays(this.defaultPagesState[this.currentPage()], this.arrayFilter(this.getChildItems()));
1144+
this.changed(_.some(this.pagesChanged));
11371145
}
11381146
});
11391147
});

0 commit comments

Comments
 (0)