Opened 10 years ago

Closed 10 years ago

#9290 closed enhancement (fixed)

Suggest adding .length property to WidgetSet instances

Reported by: dante Owned by: dante
Priority: high Milestone: 1.4
Component: Dijit Version: 1.3.0
Keywords: Cc:
Blocked By: Blocking:

Description

Because WidgetSet doesn't return a real (or subclassed) array like NodeList, it is overly verbose to do the most simple of tasks: determine if there are any matches.

eg:

var sublist = dijit.registry.filter(function(w){ return w.value == someinput.value });

to determine if there are any instances in the sublist new WidgetSet, you'd have to make a new variable, run .forEach and increment the variable, then check that.

var count = 0;
var sublist = (...).forEach(function(w){ count++ });
if(count){ ... }

Seems convoluted, but by maintaining a .length member on the instance, the above can be reduced to:

if(dijit.registry.filter(function(w){ return w.value == input.value }).length){
  // we pass the 'duplicate value' test!
}

Attached is an ubersimple changeset adding in the functionality. By utilizing the existing .add and .remove api's (the only way to manipulate the widgetSet) the task is trivial.

Will update campus docs to include addition pending approval.

Attachments (2)

length.patch (1.4 KB) - added by dante 10 years ago.
registry.patch (10.5 KB) - added by dante 10 years ago.
updated with some, every and toArray

Download all attachments as: .zip

Change History (12)

Changed 10 years ago by dante

Attachment: length.patch added

comment:1 Changed 10 years ago by bill

Owner: changed from bill to dante

Sure, sounds fine, go for it. I guess there should be a unit test too though. See also #4122.

comment:2 Changed 10 years ago by bill

PS: although it would be better (at least for your example) to just have some() and every() methods.

comment:3 Changed 10 years ago by dante

uhh, why is [1,2,3].splice(-1) returning empty array in IE6?

comment:4 Changed 10 years ago by dante

nm, patch updated. Meant to use [].slice(-1)

comment:5 Changed 10 years ago by dante

Milestone: tbd1.4
Status: newassigned

will add .every and .some and tests, and commit. Thanks for the review @bill

Changed 10 years ago by dante

Attachment: registry.patch added

updated with some, every and toArray

comment:6 Changed 10 years ago by Les

I have a minor improvement suggestion:

remove: function(/*String*/ id){ 	 
	//              Remove a widget from this WidgetSet. Does not destroy the widget; simply 
	//              removes the reference. 
	if(this._hash[id]){
		delete this._hash[id]; 
		this.length = Math.max(0, this.length - 1); 
	}
},

comment:7 Changed 10 years ago by dante

if we add the if(this._hash[id]) the Math.max() is not required (it will never decrement unless the id is found) ... seems leaving out the if() check would be adequate in this case, but yes, alex had suggested max(0, --length) so it can never reach -1

comment:8 Changed 10 years ago by Les

I was trying to prevent a situation where calling remove() with an id of a widget that was already removed would decrement length. If there's no if(), it's possible to decrease length w/o removing a widget by making multiple remove() calls with the same id parameter.

comment:9 Changed 10 years ago by dante

@Les - good point. I should have considered that. Will adjust.

comment:10 Changed 10 years ago by dante

Resolution: fixed
Status: assignedclosed

(In [17529]) fixes #9290 - adding .length property, .some and .every functions, makes .forEach return instance, allows thisObj passed to forEach some and every, adding toArray method as well as .map \!strict

Note: See TracTickets for help on using tickets.