Skip to content

Loading…

arrays: remove() without copy and name array.removeWithCopy() #58

Closed
wants to merge 2 commits into from

3 participants

@Krinkle

The default should be assumed to not make a copy. Making a copy is a special case (or it should be called "without" instead or "remove").

See also issue #55.

@rmurphey rmurphey commented on an outdated diff
tests/app/arrays.js
((7 lines not shown))
});
- it("you should be able to remove a value from an array, returning the original array", function() {
+ it("you should be able to remove a value from an array, without changing the original array", function() {
@rmurphey Owner
rmurphey added a note

How about

it("you should be able to return a copy of an array with certain values removed", function() {
@Krinkle
Krinkle added a note

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rmurphey
Owner

This should address #55. It would be great to get a pull request on the answers repo as well so that they stay in sync.

@Krinkle

Could you merge pull #56 first?

Krinkle added some commits
@Krinkle Krinkle tests/arrays: Assert that result does not equal original. 270313c
@Krinkle Krinkle arrays: remove() without copy and name array.removeWithCopy()
The default should be assumed to not make a copy. Making
a copy is a special case (or it should be called "without"
instead or "remove").
f87b018
@Krinkle

See #59.

@Krinkle Krinkle closed this
@beeeswax

I was just about to submit a pull request for this same change! :smile:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 3, 2013
  1. @Krinkle
  2. @Krinkle

    arrays: remove() without copy and name array.removeWithCopy()

    Krinkle committed
    The default should be assumed to not make a copy. Making
    a copy is a special case (or it should be called "without"
    instead or "remove").
Showing with 8 additions and 5 deletions.
  1. +1 −1 app/arrays.js
  2. +7 −4 tests/app/arrays.js
View
2 app/arrays.js
@@ -14,7 +14,7 @@ define(function() {
},
- removeWithoutCopy : function(arr, item) {
+ removeWithCopy : function(arr, item) {
},
View
11 tests/app/arrays.js
@@ -27,20 +27,23 @@ define([
expect(result).to.have.length(3);
expect(result.join(' ')).to.eql('1 3 4');
+
+ // make sure that you return the same array instance
+ expect(result).equal(a);
});
- it('you should be able to remove a value from an array, returning the original array', function() {
+ it('you should be able to return a copy of an array with certain values removed', function() {
a.splice( 1, 0, 2 );
a.push( 2 );
a.push( 2 );
- var result = answers.removeWithoutCopy(a, 2);
+ var result = answers.removeWithCopy(a, 2);
expect(result).to.have.length(3);
expect(result.join(' ')).to.eql('1 3 4');
- // make sure that you return the same array instance
- expect(result).equal(a);
+ // make sure that you didn't change the original array instance
+ expect(result).not.to.equal(a);
});
it('you should be able to add an item to the end of an array', function() {
Something went wrong with that request. Please try again.