Unnecessary call to ensureIdUniqueness in slick.dataview.js:setItems

[turchanov]

Hi!

It really seems that a call to ensureIdUniqueness right after updateIdxById() is superfluous in function setItems

SlickGrid/slick.dataview.js

Lines 130 to 139 in 427e2bf

	function setItems(data, objectIdProperty) {
	if (objectIdProperty !== undefined) {
	idProperty = objectIdProperty;
	}
	items = filteredItems = data;
	idxById = {};
	updateIdxById();
	ensureIdUniqueness();
	refresh();
	}

because when the function updateIdxById traverses items, it checks that every data item has id property defined and builds idxById array

SlickGrid/slick.dataview.js

Lines 104 to 114 in 427e2bf

	function updateIdxById(startingIndex) {
	startingIndex = startingIndex || 0;
	var id;
	for (var i = startingIndex, l = items.length; i < l; i++) {
	id = items[i][idProperty];
	if (id === undefined) {
	throw new Error("Each data element must implement a unique 'id' property");
	}
	idxById[id] = i;
	}
	}

and the function ensureIdUniqueness yet again checks checks that every data item has id property defined and verifies that idxById references that item (which is true by construction in updateIdxById)

SlickGrid/slick.dataview.js

Lines 116 to 124 in 427e2bf

	function ensureIdUniqueness() {
	var id;
	for (var i = 0, l = items.length; i < l; i++) {
	id = items[i][idProperty];
	if (id === undefined || idxById[id] !== i) {
	throw new Error("Each data element must implement a unique 'id' property");
	}
	}
	}

I would rather remove that unnecessary call to ensureIdUniqueness and the definition of that function since it is not used anywere else. I can push a pull request if you OK that change.

[ghiscoding]

closing since after trying the suggestion, the error is not thrown when we really have duplicate, so unfortunately the suggestion doesn't seem to be valid

[6pac]

I think the issue is that updateIdxById assigns the reverse lookup and if there are duplicates, the last one wins and the previous references to that ID are overwritten. ensureIdUniqueness goes through and checks that the reverse lookups resolve correctly, which they won't if something has been overwritten.