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.

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

something is possibly wrong with addElementToBeginningOfArray and addElementToEndOfArray tests #103

Open
bboroda opened this issue Sep 15, 2016 · 3 comments

Comments

@bboroda
Copy link
Collaborator

bboroda commented Sep 15, 2016

I used the same implementation for both addElementToBeginningOfArray and destructivelyAddElementToBeginningOfArray functions and I passed the tests (in other words, the test didn't catch that I was modifying the array with unshift operation).

Original implementation that passed arrays-test.js:

function addElementToBeginningOfArray(array, element) {
array.unshift(element);
return array;
}

function destructivelyAddElementToBeginningOfArray(array, element) {
array.unshift(element);
return array;
}

Notice it's the same implementation, but here is the to-do from the README.md file

TODO: In arrays.js, define two functions, addElementToBeginningOfArray and destructivelyAddElementToBeginningOfArray. Both functions take two parameters, an array and an element to add to the beginning of the array, and both functions should add the element to the beginning of the array and then return the whole array. The destructive function, destructivelyAddElementToBeginningOfArray, should alter the original array that's passed in; addElementToBeginningOfArray, on the other hand, should return a new array and not modify the original.

@albinovulcan
Copy link
Collaborator

Yeah, there are bugs in those two tests, line 29 and line 57.

The expect(array).to.eql(array) will always pass - the implementation should be expect(array).to.eql([1]) in both tests.

@albinovulcan
Copy link
Collaborator

Heh, I have no idea if a pull request is appropriate here to fix.

@koko
Copy link

koko commented Jul 23, 2017

I fixed them in my pull request, if it is appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants