Ticket #2531655 (assigned enhancement)

Reporter


John Lindal
Opened: 01/4/12
Last modified: 08/22/13
Status: assigned
Type: enhancement

Owner


Jenny Donnelly
Target Release: 3.CURRENT.NEXT
Priority: P3 (normal)
Summary: Please make oop.js:dispatch() public
Description:

I need it for my Functional Programming Gallery module. Currently, I copied it, but it would be nicer if I could use the existing version in YUI.

Type: enhancement Observed in Version: 3.4.1
Component: YUI Global Object Severity: S4 (low)
Assigned To: Jenny Donnelly Target Release: 3.CURRENT.NEXT
Location: Library Code Priority: P3 (normal)
Tags: Relates To:
Browsers: N/A
URL:
Test Information:

Change History

Dav Glass

Posted: 01/4/12
  • location changed to Library Code
  • owner changed from Dav Glass to Ryan Grove
  • priority changed to P3 (normal)
  • status changed from new to assigned

Ryan Grove

YUI Developer

  • Username: rgrove
  • GitHub: rgrove
Posted: 01/4/12
  • status changed from assigned to infoneeded

I feel kinda weird about this, since I really don't like dispatch() and have actually been hoping to get rid of it. It's only used by Y.each() and Y.some(), and I think we should discourage the use of those methods and instead encourage people to use the appropriate methods on Y.Array or Y.Object. Can you convince me otherwise?

John Lindal

YUI Contributor

  • Username: jafl
  • GitHub: jafl
Posted: 01/4/12
  • status changed from infoneeded to assigned

Forcing people to call Y.Array or Y.Object explicitly prevents people from writing fully generic code, i.e., code that can accept an array, an object, or a NodeList. Here is an updated version of dispatch which works even for reduce(), which has a different set of arguments:

function dispatch(action, o)
{

var args = Y.Array(arguments, 1, true);

if (o && o[action] && o !== Y)
{
args.shift();
return o[action].apply(o, args);
}

switch (Y.Array.test(o))
{
case 1:
return Y.Array[action].apply(null, args);
case 2:
args[0] = Y.Array(o, 0, true);
return Y.Array[action].apply(null, args);
default:
return Y.Object[action].apply(null, args);
}
}

John Lindal

YUI Contributor

  • Username: jafl
  • GitHub: jafl
Posted: 01/4/12

Actually, that doesn't work for reduce() on arrays. This does:

function dispatch(action, o)
{

var args = Y.Array(arguments, 1, true);

switch (Y.Array.test(o))
{
case 1:
return Y.Array[action].apply(null, args);
case 2:
args[0] = Y.Array(o, 0, true);
return Y.Array[action].apply(null, args);
default:
if (o && o[action] && o !== Y)
{
args.shift();
return o[action].apply(o, args);
}
else
{
return Y.Object[action].apply(null, args);
}
}
}

Ryan Grove

YUI Developer

  • Username: rgrove
  • GitHub: rgrove
Posted: 01/4/12
  • status changed from assigned to accepted

Right, I want to encourage people not to use the generic Y.each() and Y.some(), because I consider them dangerous.

When you treat arrays and objects generically, it's easy to forget that they behave differently. Relying on objects to enumerate properties in insertion order is dangerous, as is assuming that all properties of an object are enumerable just as all items in an array are iterable. On top of this there's the additional function call overhead, which makes Y.each(), Y.some(), and dispatch() less ideal in performance critical situations.

Explicitly calling Y.Array.each(), Y.Object.each(), etc. reinforces that you're working with either an object or an array, and that the two aren't interchangeable in spite of their similarities. I feel like that's a good thing.

Ryan Grove

YUI Developer

  • Username: rgrove
  • GitHub: rgrove
Posted: 01/4/12
  • status changed from accepted to infoneeded

Bleh, didn't mean to accept. You'll still need to convince me. :)

John Lindal

YUI Contributor

  • Username: jafl
  • GitHub: jafl
Posted: 01/4/12
  • status changed from infoneeded to assigned

You're right that arrays and objects work differently, but arrays and NodeLists work exactly the same.

Ryan Grove

YUI Developer

  • Username: rgrove
  • GitHub: rgrove
Posted: 01/4/12
  • status changed from assigned to accepted

Yep. I'm not opposed to having all the Y.Array methods support NodeLists and other array-like objects. I just think treating arrays and objects the same is a bad idea.

Ryan Grove

YUI Developer

  • Username: rgrove
  • GitHub: rgrove
Posted: 01/4/12
  • resolution changed to wontfix
  • status changed from accepted to closed

Cripes, I hate that I'm forced to automatically accept things when I comment on them.

Closing wontfix, but I'll definitely address the other bug you filed re. adding support for array-like objects in Y.Array.

John Lindal

YUI Contributor

  • Username: jafl
  • GitHub: jafl
Posted: 01/5/12
  • resolution changed from wontfix
  • status changed from closed to reopened

While I agree that in some cases, iteration *may* matter for Y.some, Y.every, and Y.reduce, the whole point of Y.each, Y.map, Y.filter, Y.reject, Y.partition is that iteration order is irrelevant. They are trivially parallelizable -- no matter that JavaScript is single threaded :)

Dav Glass

Posted: 05/15/12
  • owner changed from Ryan Grove to Dav Glass
  • status changed from reopened to assigned

Dav Glass

Posted: 05/15/12
  • status changed from assigned to accepted

John Lindal

YUI Contributor

  • Username: jafl
  • GitHub: jafl
Posted: 05/15/12

Pull request coming up :)

Dav Glass

Posted: 05/15/12
  • owner changed from Dav Glass to Satyen Desai
  • status changed from accepted to assigned

John Lindal

YUI Contributor

  • Username: jafl
  • GitHub: jafl
Posted: 05/15/12

John Lindal

YUI Contributor

  • Username: jafl
  • GitHub: jafl
Posted: 05/15/12

Sorry, missed the reassignment. Here is the pull request for yui: https://github.com/yui/yui3/pull/139

John Lindal

YUI Contributor

  • Username: jafl
  • GitHub: jafl
Posted: 05/15/12

Note that I would actually like to see gallery-funcprog, gallery-object-extras, etc merged into YUI :)

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 09/24/12
  • milestone changed to 3.CURRENT.NEXT
  • status changed from assigned to accepted

As mentioned in the pull request, will resolve this one way or the other, in this upcoming sprint.

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 09/24/12
  • milestone changed from 3.CURRENT.NEXT to 3.NEXT

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 10/16/12
  • estimated changed from 0 to 0.5
  • remaining changed from 0 to 0.5
  • sprint changed to Sprint 02

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 10/16/12
  • sprint changed from Sprint 02 to Sprint 03

Messed up the sprint number. Should be sprint 3 [ current sprint ]

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 10/16/12
  • milestone changed from 3.NEXT to 3.CURRENT.NEXT

Satyen Desai

YUI Developer

  • Username: sdesai
  • GitHub: sdesai
Posted: 10/24/12
  • sprint changed from Sprint 03

Not going to get to this for this sprint. Someone else may pick it up.

Eric Ferraiuolo

YUI Developer

  • Username: ericf
  • GitHub: ericf
Posted: 01/9/13

While we're thinking about Y.Array.each supporting NodeLists, we should make sure that means it supports ArrayLists, and ModelLists.

Jenny Donnelly

YUI Developer

  • Username: jenny
  • GitHub: jenny
Posted: 08/22/13
  • owner changed from Satyen Desai to Jenny Donnelly
  • status changed from accepted to assigned