CrowdHailer / fn.js

A JavaScript library built to encourage a functional programming style & strategy. - http://eliperelman.com/fn.js
MIT License
399 stars 30 forks source link

Handling of corner cases for 'fn.concat' #17

Closed amoilanen closed 10 years ago

amoilanen commented 10 years ago

fn.concat(6, [1, 2, 3]) or fn.concat() worked differently from how I would expect them to work. In both cases an exception would be thrown. At the same time it is perfectly fine to call standard methods like arr1.concat(6) and arr1.concat(), in the first case it will just append 6 and in the second case will leave the array unchanged.

I added a few test cases covering those corner cases and modified the implementation of fn.concat a bit to handle them.

Basically I just looked at the following examples for Array.prototype.concat and String.prototype.concat and made sure that the behavior is similar for fn.concat:

var arr1 = [1, 2, 3];
var arr2 = [4, 5, 6];

console.log(arr1.concat()); //[1, 2, 3]
console.log(arr1.concat(null)); //[1, 2, 3, null]
console.log(arr1.concat((void 0))); //[1, 2, 3, undefined]
console.log(arr1.concat("4")); //[1, 2, 3, "4"]
console.log(arr1.concat(arr2)); //[1, 2, 3, 4, 5, 6]

var str1 = "123";
var str2 = "456";

console.log(str1.concat(str2)); //"123456"
console.log("".concat(str2)); //"456"
console.log(str1.concat("")); //"123" 

By the way, I assumed that fn.concat should also work with strings, not only arrays.

Please, let me know if you have any notes or suggestions.

eliperelman commented 10 years ago

This PR has bitrotted with the 0.8.0 released. Please rebase and refactor, and resubmit for review (wow that was a lot of "re"s :smile:)

amoilanen commented 10 years ago

I merged the recent changes :) Can you, please, take a look?

eliperelman commented 10 years ago

To me this was a bug in my implementation. I have merged your changes and tagged this for release v0.8.1. Thanks for your contribution!

amoilanen commented 10 years ago

OK. Thank you!