Higher-order functions accepting Arrays or Objects

[zship]

I'd like to see the higher-order functions under "array/" be available for Arrays or Objects, similar to Underscore's "Collection" functions. I think sometimes it's convenient to accept Array | Object in method parameters. Async.js is a good example of a library where this convention is useful. I'm currently writing this (re-using amd-utils modules where appropriate) and calling it "collection/". Would you want to pull it?

[millermedeiros]

sure! what about creating a new package collection while keeping all the array/object methods separate? eg:

// collection/filter.js
define(['../lang/isArray', '../array/filter', '../object/filter'], function(isArray, arrFilter, objFilter){

  function filter(target, callback, thisObj){
     return isArray(target)? arrFilter(target, callback, thisObj) : objFilter(target, callback, thisObj);
  }

  return filter;

});

I think we should keep it as backwards compatible as possible (diff projects rely on amd-utils) and there is a perf hit by calling isArray() and by loading both methods, so it's good to keep it optional.

This process could be automated by the build tool or by a macro. - yesterday I added the option to create new modules through the build script node build add collection/filter (will create a stub module, update packages and create a failing test).

What do you think?

[millermedeiros]

Uhm, maybe we could abstract it into a separate module if the logic is always the same (which should be):

// collection/make_.js
define(['../lang/isArray'], function(isArray){

    function makeCollectionMethod(arrMethod, objMethod) {
        return function(){
            var args = Array.prototype.slice.call(arguments);
            return isArray(args[0])? arrMethod.apply(null, args) : objMethod.apply(null, args);
        };
    }

    return makeCollectionMethod;

});

// collection/filter.js
define(['./make_', '../array/filter', '../object/filter'], function(make, arrFilter, objFilter){

    return make(arrFilter, objFilter);

});

Keep it DRY.

[zship]

Yeah, I'm on board with keeping it a separate module. The performance hit should be expected, I think, since by using it you're essentially declaring "I don't know what you're passing me!".

Your comment about using a generic makeCollectionMethod method is an idea that never occurred to me. Although, if I'm remembering right, your object package doesn't currently contain all of the common higher-order functions (checking it now, every and reduce are a few that are missing). Now that you mention it, though, maybe it would be a better idea to add those to object and then use makeCollectionMethod for the entire collection package. I already wrote the collection package, but I like your idea better. Done that way, users could get a modest performance gain if they know they're dealing with only Objects or Arrays, and use collection otherwise.

[millermedeiros]

Yes, we would need to add the missing methods to the object package, I've been adding new stuff as needed but the goal is to have all the important methods from underscore.js/lodash + many more.

Maybe we should start with the most common methods like forEach, map, filter and after we have a clear structure of how the tests would work - probably we just need 1 or 2 tests for each method, only to check if the result contains what we expect and is of the proper type (array or object) - we add the missing ones.

I will update the build tool to generate the collection packages automatically, thinking in something like node build add collection/filter --collection would generate the module I sent on my previous comment automatically and also a failing spec for this module.

[zship]

I started a fork with a "collection" branch for this: https://github.com/zship/amd-utils/tree/collection . Added the missing object methods. Also added a size method to the "collection" package, as that wouldn't be auto-generated (arrays don't need a size method). With your generation script, we should be in business! I'm new to writing unit tests, but I'll see if I can write some for the new object methods (that's probably the harder part of this, huh?).

[millermedeiros]

I added the command to automatically create the collection module node build add collection/filter -c and added tests for them as well. We surely need proper tests for all the modules to avoid regressions and cover edge cases.

To run the tests locally open the tests/specRunner.html. To make sure all modules have tests (and also to automatically update the specRunner) run node build pkg, this will update all packages and create missing spec files. Use the array specs as reference.

Objects have some weird behavior on IE if they contain one of these properties so it's better to use object/forOwn and object/forIn whenever possible. I created a new issue to add support for exiting iteration early by returning false (see #94). I should implement it soon (probably today).

Tests and documentation takes a lot of time. Luckily these methods have a lot of things in common with the array ones so we can just copy examples/specs and add links to the original methods, see: http://millermedeiros.github.com/amd-utils/collection.html

Thanks for the feedback and for your work. Keep it coming.

[zship]

Sounds great. Thanks for your fast responses to this feature request. I'll be at work until later tonight and then I'll have time to work on tests and documentation. Of course, if you get the inclination to work on that during the day, I won't complain! Otherwise, so we're not duplicating effort, I'll assume that's on me.

[jdalton]

Collection methods like those found in Lo-Dash/Underscore treat array-like objects as arrays too. They use a basic check of collection.length === +collection.length or typeof collection.length == 'number'. They also allow falsey values to be passed as collection and treat it as an empty collection.

This allows for things like _.each(string.match(regexp), callback), as String#match may return null and it allows for jQuery/other-lib collections, which are array-like objects, to be iterated.

[jdalton]

Also they iterate over objects own properties, so using your forOwn instead of forIn method in your collection's forEach.

[millermedeiros]

@jdalton makes a lot of sense. I will create proper specs and update the code accordingly. Thanks.

[zship]

Something's occurred to me while I've been working on the tests today: some current object methods in amd-utils (let's take map, for example) return Objects. I duplicated this behavior in the object methods I added (for find, pluck, and reject). In collection methods, it would probably make more sense to return a common type. Otherwise users would have to perform type checking when using collection methods (and they might as well go with the object and array equivalents at that point). I'd go with Array since maintaining ordering information when using Array input is often necessary. Underscore seems to go that route. Thoughts?

There are two avenues I see here: 1) Change object methods like map to return Array, and lose the original keys. 2) Don't use the _make method for these collection methods: filter, find, map, pluck, and reject; and always build Arrays when iterating 2 seems like the better option since original keys are sometimes useful to have.

[millermedeiros]

I agree with approach #2. I also added an option on _make to set value to be returned in case user pass null (empty array or null in most cases)

[millermedeiros]

"collection" package was released on v0.7.0