added support for variable-depth flatten

[msmorgan]

This commit adds an optional maxDepth argument to _.flatten, allowing a nested array to be recursively flattened to a variable degree.

Please double-check I made the correct change in fp/_mapping.js, because it was hard to tell what was going on in that file.

[msmorgan]

I forgot to save a file so I'm closing and re-submitting this.

[jfmengels]

You can force-push your branch instead of closing this one.

[msmorgan]

Done. Didn't know you could do that.

[jdalton]

Could you remove the dist/ and doc/ files.

[jfmengels]

I'm pretty sure you should not commit the dist/ and doc/ folders.
As for the mapping, looks good, but it depends on the behavior that you want. If you put it in the aryMethod[1] category, then you won't be able to use the maxDepth option out of the box. If you put it in the aryMethod[2], then you will be forced to add a maxDepth value every time.

I'm guessing the common case is that people want it to be flattened only by one level, and would use flattenDeep otherwise. I'd argue it would actually make more sense to have the maxDepth option in flattenDeep function (then put that one in the 2-args mapping, and flatten in the 1 or keep it uncapped).

[jdalton]

The min depth should be 1. A flatten that doesn't flatten isn't a flatten.

[msmorgan]

I intentionally made it callable with maxDepth == 0 for flexibility purposes. It defaults to 1. I'll change it if you think it's necessary.

[jdalton]

Ya it shouldn't go below 1.

[megawac]

didn't look too closely, but is the case needed for recursion?

[msmorgan]

It's not, since it won't be called recursively once maxDepth reaches 1.

[jdalton]

@megawac

didn't look too closely, but is the case needed for recursion?

Naw just base functionality flatten should flatten at least 1 level.
I've expected a depth level request for a while :P

[msmorgan]

If the consensus is that I should move this functionality to flattenDeep (with a default maxDepth value of Infinity?), I can change it tomorrow.

[jdalton]

@msmorgan

If the consensus is that I should move this functionality to flattenDeep (with a default maxDepth value of Infinity?), I can change it tomorrow.

I think that a separate method is the way to go. I like _.flatten because it's simple and doesn't have any extra params that muck up iteration. In v4 I've tried to split functionality into separate methods like _.flatten and _.flattenDeep.

I like the modification to baseFlatten but, if this is added, I imagine it would be exposed to the user as_.flattenDepth or something.

Have you signed our cla?

[msmorgan]

I just signed it (submitted a CLA on 2016-02-09).

If this will be a separate method, what do you think of the name _.flattenBy?

[jdalton]

@msmorgan

If this will be a separate method, what do you think of the name _.flattenBy?

No the "By" postfix has another meaning. I think _.flattenDepth is fine.

[msmorgan]

@jdalton

Okay, _.flattenDepth it is.

What should it do if a depth of 0 is passed? I think it should simply copy the array. Wanting to recursively flatten 0 times might not come up that often, but if it does for someone, that behavior seems correct/consistent.

However, I concede that baseFlatten should probably default to 1 if non-numeric data is passed, rather than 0 as I have it now.

[jdalton]

What should it do if a depth of 0 is passed?

Okaaaay, it's gross but consistent to do a simple arrayCopy.
I checked and other methods that default to n of 1 still allow 0 and you get what you get.

[StreetStrider]

flattenTo?
flattenUnder?
flattenAt (at level)?

[jdalton]

I like flattenTo but we already have a pattern of taking a param and making it part of the name

• xyzDeep
• xyzSize
• xyzN

So turning in to _.flattenDepth is fine

[msmorgan]

I've moved the new functionality into a separate _.flattenDepth function.

Please verify that I am defaulting the depth argument to baseFlatten correctly.

[jdalton]

rogue debugger

[msmorgan]

Whoops! Should have noticed that.

At one point, I had a bug where baseFlatten was defaulting depth to 0 instead of 1 (because toInteger(undefined) behaved differently than I assumed), and the test suite wouldn't even load.

[msmorgan]

New commit fixes those three things.

[jdalton]

You can move the depth < 1 and copyArray() bit to flattenDepth.

[jdalton]

Thank you @msmorgan!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.