changed == to === and formatted to jslint standards.

[jlaustill]

I'm VERY new to git and github, I've been developing in C# and using svn and mercurial for a very long time. So I decided to pick a project that I could just work on getting more familiar with and try to help with small improvements. If this isn't how you like to work, or if you'd rather I go learn somewhere else that won't hurt my feelings at all :)

All I did here was pick the smallest function I could find in your code and format it to pass a jslint test, then made sure all of your 100 specs still ran successfully. Not a huge improvement :)

[crissdev]

I would leave the check for null as it was before just to prevent the function from failing on undefined value.

[jlaustill]

That is a valid concern, but I assumed the null value was the direct intention. If it's not, I would ask if it should also be checking for Nan and false? The == comparison won't detect those.

Just checking for a truthy match would replace a numerical 0 with an empty string if I'm not mistaken, and since the next statement checks for numerals and converts them to strings, I assume that is an unwanted behavior as well.

I would rather see if string === null || string === undefined myself if that is the intention.

[jlaustill]

Ok, I went through this as thoroughly as I could and created specs for this function. Which unsurprisingly brought up more questions :) The regex for the replacement after casting to string is pretty straight forward, but I couldn't for the life of me think of what object would be cast to string that would contain a non breaking space that needed trimmed. If you can give me an example I would gladly add the test for it as this one kind of has me stumped.

Also, currently this function will return the string "NaN" if you pass in NaN, and that seems to be the case with == null or === null. Is this expected behavior? I would expect maybe a trim function to throw on that as it would most likely indicate a serious issue elsewhere in the code, but maybe that isn't the case and it should just return an empty string? I'm curious on your thoughts.

In any rate, tweak these tests/specs and give me the example from above and I will gladly make this function jslint quality that satisfies all tests. I love puzzles like this.

[mbest]

string === null || string === undefined is the intention.

[jlaustill]

Updated to reflect as such :) All tests passing.

[mbest]

undefined and null are the only values for which you can't call toString(). They also usually have a meaning of "unset", which is generally displayed as blank.

[jlaustill]

Awesome, that's exactly why I'm doing this, to learn stuff like that. I'll have to research unset a bit.

[mbest]

I'm speaking mostly of convention within JavaScript apps, especially with Knockout. Even a variable that will generally hold a string may be declared without being initialized and so have a value of undefined.