Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add _.transform method #1901

Merged
merged 1 commit into from
Dec 29, 2014
Merged

Add _.transform method #1901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions test/collections.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,53 @@
strictEqual(_.reduceRight, _.foldr, 'alias for reduceRight');
});

test('transform', function() {
var list = _.transform(['foo', 'bar', 'baz'], function(accumulator, value, index){ accumulator[index] = value + '2'; });
deepEqual(list, ['foo2', 'bar2', 'baz2'], 'handles arrays with no accumulator');

list = _.transform(['foo', 'bar', 'baz'], function(accumulator, value, index){ accumulator[index] = value + '2'; }, []);
deepEqual(list, ['foo2', 'bar2', 'baz2'], 'handles arrays with array accumulator');

list = _.transform(['foo', 'bar', 'baz'], function(accumulator, value, index){ accumulator[index] = value + '2'; }, {});
deepEqual(list, {0: 'foo2', 1: 'bar2', 2: 'baz2'}, 'handles arrays with object accumulator');

var obj = _.transform({foo: 1, bar: 2, baz: 3}, function(accumulator, value, key){ accumulator[key] = value + 1; });
deepEqual(obj, {foo: 2, bar: 3, baz: 4}, 'handles objects with no accumulator');

obj = _.transform({foo: 1, bar: 2, baz: 3}, function(accumulator, value, key){ accumulator[key] = value + 1; }, {});
deepEqual(obj, {foo: 2, bar: 3, baz: 4}, 'handles objects with array accumulator');

obj = _.transform({0: 'foo', 1: 'bar', 2: 'baz'}, function(accumulator, value, key){ accumulator[key] = value + '2'; }, []);
deepEqual(obj, ['foo2', 'bar2', 'baz2'], 'handles objects with object accumulator');

function Obj(props) { _.extend(this, props); }
var instance = new Obj({foo: 1, bar: 2, baz: 3});
obj = _.transform(instance, function(accumulator, value, key){ accumulator[key] = value + 1; });
ok(obj !== instance && obj instanceof Obj, 'creates a new instance of the object');
deepEqual(obj, new Obj({foo: 2, bar: 3, baz: 4}), 'handles instances with no accumulator');

var context = {}, actualContext;
_.transform([1], function() { actualContext = this; }, {}, context);
strictEqual(actualContext, context, 'iterates with the correct context');

obj = {foo: 1};
var accumulator = {};
var args = [accumulator, 'foo', obj.foo, obj], actualArgs;
_.transform(obj, function() { actualArgs = _.toArray(args); }, accumulator);
deepEqual(args, actualArgs, 'iterates with the correct arguments');

accumulator = {};
strictEqual(_.transform([1], function() {}, accumulator), accumulator);

deepEqual(_.transform(), {}, 'should return an empty object when no obj or accumulator is passed');

accumulator = [];
strictEqual(_.transform(null, null, accumulator), accumulator, 'should return the accumulator when no obj is passed');

list = _.transform([1, 2, 3, 4], function(accumulator, value) { return value < 3 && accumulator.push(value); });
deepEqual(list, [1, 2], 'transform should stop when false is returned.');
});

test('find', function() {
var array = [1, 2, 3, 4];
strictEqual(_.find(array, function(n) { return n > 2; }), 3, 'should return first found `value`');
Expand Down
25 changes: 25 additions & 0 deletions underscore.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,31 @@
return memo;
};

// **Transform** is an alternative to reduce that transforms `obj` to a new
// `accumulator` object.
_.transform = function(obj, iteratee, accumulator, context) {
if (accumulator == null) {
if (_.isArray(obj)) {
accumulator = [];
} else if (_.isObject(obj)) {
var Ctor = obj.constructor;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this conditional path really necessary? If the dev wants the return to be an instance of obj's class, why not leave it to them to pass in the proper accumulator?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it adds support for other native types as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meaning the typed arrays? If so, we might be able to do a good enough job by reordering the obj and accumulator conditionals:

if (obj == null) return accumulator != null ? accumulator : {};
if (accumulator == null) accumulator = obj.length === +obj.length ? [] : {};

It would generalize array-likes to an array, leaving it up the dev to be more specific if they want it.

I'm not particularly against this, I just think grabbing .constructor.prototype is ugly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypedArrays, Map, Set, etc., including anything created in user-land as well. Inferring the type is one of the things that makes transform so nice.

_.transform(new Backbone.Model, function() { /* ... */ }) instanceof Backbone.Model // >> true

I agree that accessing .constructor.prototype isn't very nice, but only if the user is doing something gross with it in the first place, in which case you can still specify the accumulator. Lodash's implementation works this way so @jdalton can speak to any problems that have come up because of it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only issue I can see with that method is constructor overrides, but thats cool by me

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdalton: Your use cases for it would be valuable.

Underscore can't iterate a Map or a Set, so it still seems like the only gain over my above comment is instanceof? I'm 50-50 for supporting it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The create use of _.transform came before the actual _.create method for me. It's a nice to have. The rationale for having it is you're transforming a value so the result should be of the same constructor. I've taken this approach in our clone method too preferring clones to be of the realm their original values. For me the biggest win is the ability to exit early and not providing a default accumulator so the create can be seen as a non-critical requirement that could be added at a later time.

accumulator = baseCreate(typeof Ctor == 'function' && Ctor.prototype);
} else {
accumulator = {};
}
}
if (obj == null) return accumulator;
iteratee = optimizeCb(iteratee, context, 4);
var keys = obj.length !== +obj.length && _.keys(obj),
length = (keys || obj).length,
index, currentKey;
for (index = 0; index < length; index++) {
currentKey = keys ? keys[index] : index;
if (iteratee(accumulator, obj[currentKey], currentKey, obj) === false) break;
}
return accumulator;
};

// Return the first value which passes a truth test. Aliased as `detect`.
_.find = _.detect = function(obj, predicate, context) {
var key;
Expand Down