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

Jump to bottom

optimize partial and partialRight to correctly process rest args #2941

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

optimize partial and partialRight to correctly process rest args #2941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8fe3c9b
optimize partial and partialRight to correctly process rest args
adispring Dec 22, 2019
9a79517
modify partial & partialRight's test
adispring Dec 23, 2019
0987d0a
Merge branch 'master' into optimize-partial-partialRight
adispring Jan 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 13 additions & 2 deletions source/partial.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import _arity from './internal/_arity.js';
import _concat from './internal/_concat.js';
import _createPartialApplicator from './internal/_createPartialApplicator.js';
import _curry2 from './internal/_curry2.js';


/**
Expand Down Expand Up @@ -30,5 +31,15 @@ import _createPartialApplicator from './internal/_createPartialApplicator.js';
* sayHelloToMs('Jane', 'Jones'); //=> 'Hello, Ms. Jane Jones!'
* @symb R.partial(f, [a, b])(c, d) = f(a, b, c, d)
*/
var partial = _createPartialApplicator(_concat);
var partial = _curry2(function partial(fn, partialArgs) {
if (partialArgs.length >= fn.length) {
return fn.apply(this, partialArgs);
} else {
var restLength = fn.length - partialArgs.length;
return _arity(restLength, function() {
return fn.apply(this, _concat(partialArgs, arguments));
});
}
});

export default partial;
19 changes: 14 additions & 5 deletions source/partialRight.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import _concat from './internal/_concat.js';
import _createPartialApplicator from './internal/_createPartialApplicator.js';
import flip from './flip.js';

import _arity from './internal/_arity.js';
import _curry2 from './internal/_curry2.js';
import insertAll from './insertAll.js';

/**
* Takes a function `f` and a list of arguments, and returns a function `g`.
Expand All @@ -27,5 +26,15 @@ import flip from './flip.js';
* greetMsJaneJones('Hello'); //=> 'Hello, Ms. Jane Jones!'
* @symb R.partialRight(f, [a, b])(c, d) = f(c, d, a, b)
*/
var partialRight = _createPartialApplicator(flip(_concat));
var partialRight = _curry2(function partialRight(fn, partialArgs) {
if (partialArgs.length >= fn.length) {
return fn.apply(this, partialArgs);
} else {
var restLength = fn.length - partialArgs.length;
return _arity(restLength, function() {
return fn.apply(this, insertAll(restLength, partialArgs, arguments));
});
}
});

export default partialRight;
12 changes: 10 additions & 2 deletions test/partial.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
var R = require('../source/index.js');
var eq = require('./shared/eq.js');


describe('partial', function() {
var disc = function(a, b, c) { // note disc(3, 7, 4) => 1
var disc = function(a, b, c) {
// note disc(3, 7, 4) => 1
return b * b - 4 * a * c;
};

Expand All @@ -21,4 +21,12 @@ describe('partial', function() {
eq(g.length, 1);
});

it('correctly processes rest of the arguments', function() {
function greet(salutation, title, firstName, lastName) {
return salutation + ', ' + title + ' ' + firstName + ' ' + lastName + '!';
}
var sayHello = R.partial(greet, ['Hello']);
var sayHelloToMs = R.partial(sayHello, ['Ms.']);
eq(sayHelloToMs('Jane', 'Jones', '&', 'Green'), 'Hello, Ms. Jane Jones!');
});
Copy link
Member

Choose a reason for hiding this comment

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

So long as we're supporting ES5, I don't think we can include this test.

Copy link
Member Author

Choose a reason for hiding this comment

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

So how should I modify the test, should these test files stay as before?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should remove the tests for ...rest and add one for the version of greet that inspired #2940. That should show the behavior well enough.

Copy link
Member Author

@adispring adispring Dec 23, 2019 •
edited

Choose a reason for hiding this comment

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

As this way?:

const greet = (salutation, title, firstName, lastName) =>
  salutation + ', ' + title + ' ' + firstName + ' ' + lastName + '!';

const greetMsJaneJones = R.partialRight(greet, ['Ms.', 'Jane', 'Jones']);

greetMsJaneJones('Hello', 'Mr.' 'Green'); //=> 'Hello, Ms. Jane Jones!'

Copy link
Member

Choose a reason for hiding this comment

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

exactly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Has changed the test as you suggested.

});
11 changes: 9 additions & 2 deletions test/partialRight.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
var R = require('../source/index.js');
var eq = require('./shared/eq.js');


describe('partialRight', function() {
var disc = function(a, b, c) { // note disc(3, 7, 4) => 1
var disc = function(a, b, c) {
// note disc(3, 7, 4) => 1
return b * b - 4 * a * c;
};

Expand All @@ -21,4 +21,11 @@ describe('partialRight', function() {
eq(g.length, 1);
});

it('correctly processes rest of the arguments', function() {
function greet(salutation, title, firstName, lastName) {
return salutation + ', ' + title + ' ' + firstName + ' ' + lastName + '!';
}
var greetMsJaneJones = R.partialRight(greet, ['Ms.', 'Jane', 'Jones']);
eq(greetMsJaneJones('Hello', 'Mr.', 'Green'), 'Hello, Ms. Jane Jones!');
});
});
Copy link
Member

Choose a reason for hiding this comment

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

Ditto