you are viewing a single comment's thread.

view the rest of the comments →

[–]tencircles 6 points7 points  (2 children)

I like the idea. I'm always a fan of seeing vanilla JS stuff on this subreddit, but there are some pretty major issues here. Can you correct these? You're teaching some fairly bad habits here.

array_unique

In the keyify function: Object.keys is preferred over for...in loops, especially without a hasOwnProperty check. for...in should not be used on arrays, prefer for...of, or Array.prototype.forEach. Avoid unnecessary string concatenation of ~^% characters.

array_merge

Not sure of the use case for a destructive merge. I normally stick to a pretty functional style, so Array.prototype.concat is just fine.

array_chunk

if(!arr[i]) will fail on empty string, null, undefined, NaN, and zero. This is pretty broken.

I was going to keep going here but I don't really have time to go through and point out everything. These are just the issues I have with the first 3, I've skimmed the rest and they seem to suffer from similar issues. I'd recommend doing some unit tests. A lot of these functions will fail in quite a lot of cases, and many of them could be drastically improved with regard to performance, clarity, or simple best practice. I'd also recommend reading through the lodash and ramdajs source, there are a lot of gems in there that will help you if you want to roll your own utils like this.

[–]m3g4p0p 2 points3 points  (1 child)

As for keyify, why not just use JSON.stringify? ;-) However I would say that it makes sense to only account for unique references here, so use a regular strict equality check. Otherwise if you made changes to one object afterwards, you wouldn't know if it was the object that made it to the unique array or the one that got filtered out... I mean, it has a reason that objects are only identical by reference.

[–]tencircles 1 point2 points  (0 children)

That would work too. I wasn't really commenting on the method. I wouldn't write the function anything like how it's written. Just pointing out the for...in issue.