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

Digitize (variadic) #160

Closed
meetzaveri opened this issue Dec 15, 2017 · 9 comments
Closed

Digitize (variadic) #160

meetzaveri opened this issue Dec 15, 2017 · 9 comments

Comments

@meetzaveri
Copy link
Contributor

meetzaveri commented Dec 15, 2017

https://github.com/Chalarangelo/30-seconds-of-code#number-to-array-of-digits

Here I found out one bug/caveat! Compare these two code,

//first code 
const digitize = n => (''+n).split('').map(i => parseInt(i));
// digitize(2334) ->  (4) [2, 3, 3, 4]
//second code
const digitize = n => (''+n).split('').map(i => parseInt(i));
//   digitize(2,3,3,4) -> [2] 0:2

So here's the bug where user wants to input in separated manner with ',' then function will not work. Also in first code, user wants to insert multiple values which has more than two digits then,

digitize(23,232,3424);
//(4) [2, 3, 3, 4]

So definitely depends how user inputs. So we should write in desc. that user should enter input in specific manner so that it returns as pure

@Chalarangelo
Copy link
Owner

Isn't the digitize function supposed to take just one argument? I mean we can convert it to work with variadic arguments, but I am not sure this is necessary. Opinions?

@meetzaveri
Copy link
Contributor Author

But what if user needs to input two digit numbers? Then it will not work as third snippet in issue is!

@Chalarangelo
Copy link
Owner

The function signature indicates that only one argument should be passed. If we want to convert to a variadic function, we should do something like (off the top of my head, test this):

const digitize = (...args) => ['',...args].join('').split('').map(i => parseInt(i));

That one should work for variadic arguments.

@meetzaveri
Copy link
Contributor Author

meetzaveri commented Dec 15, 2017

Then also whole two digit number is not going to be selected in array.

const digitize = (...args) => ['',...args].join('').split('').map(i => parseInt(i));
digitize(12,123,1234);
// (9) [1, 2, 1, 2, 3, 1, 2, 3, 4] , should be [12,123,1234]

@Chalarangelo
Copy link
Owner

Not at all, the result is exactly as intended. It's suppose to split a number (or numbers) into an array of digits.

@meetzaveri
Copy link
Contributor Author

I thought it should be like supporting two digit value also.

No problem if the main reason is to split out a single digit number.

@Chalarangelo
Copy link
Owner

The original idea was to split one number into digits (which has valid use-cases as far as I can tell, had to do this in the past). I don't know if we have to make this variadic, as I can't remember the last time I had to digitize a bunch of numbers in the same array, plus you can digitize multiple values by sticking them in an Array and calling Array.map() so I vote we keep as-is.

@Chalarangelo Chalarangelo changed the title [Bug] : Number to array has problem with user input Digitize (variadic) Dec 15, 2017
@meetzaveri
Copy link
Contributor Author

meetzaveri commented Dec 15, 2017

Okay got it about the original idea !
sure you can now close this or maybe depend for more suitable outcome!

@lock
Copy link

lock bot commented Dec 18, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for any follow-up tasks.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants