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

Shorter version of kata 51 "generator:yield" test case #2 #12

Closed
tobireif opened this issue Jul 20, 2015 · 8 comments
Closed

Shorter version of kata 51 "generator:yield" test case #2 #12

tobireif opened this issue Jul 20, 2015 · 8 comments

Comments

@tobireif
Copy link
Contributor

Not an issue/bug, just an offer:

I could change (via PR) the initial code
from
let thisStep = generator;
let [value, done] = [thisStep.value, thisStep.done];
to shorter
let {value, done} = generator;
.

@wolframkriesing
Copy link
Collaborator

that would work, i tried it. though it adds an extra layer of complexity, which is that the user HAS TO understand destructuring (which should also be mentioned in the metadata then) ... do you think this is acceptable, when you want to learn "just" how a generator works?

@JonathanPrince
Copy link
Contributor

i personally think that the longer version is more helpful in the context of a learning tool, also since the following tests build on the same structure.

@tobireif
Copy link
Contributor Author

The current version of the test case also has destructuring, right? :)
My version features shorter but slightly more complex destructuring though.

If you require knowledge of destructuring (which is OK, if destructuring gets demonstrated in an earlier kata), you could use my shorter code if you want.
(I could create a PR, where also the next test case could be shortened to

generator.next();
let {value, done} = generator;

)

Or if no knowledge of destructuring should be required, at all, you might want to consider getting rid of the array-based destructuring assignment as well.

@wolframkriesing
Copy link
Collaborator

I would not assume any certain order of how the katas are done. therefore keeping the complexity simple is a plus imho. This might change over the next year, and if destructuring really has gotten stuck in all people's minds, this kata might use it more hard-core, but not yet.
thanks for the input anyways, I like those discussions. Also to see and be able to articulate better what I see as important in katas.

@tobireif
Copy link
Contributor Author

If you don't want to require any knowledge of destructuring (neither object- nor array-based desctructuring), you'd have to get rid of the current array-based destructuring assignment as well, I think.

From current code

let thisStep = generator;
let [value, done] = [thisStep.value, thisStep.done];

to

let thisStep = generator;
let value = thisStep.value;
let done = thisStep.done;

I could create a PR.

Thanks for the katas!

@wolframkriesing
Copy link
Collaborator

what does @jonathan think? I think this one is simple enough to understand,
the const {value, done} = generator.next() would be less
readable/accessible and add a hurdle, in the current solution, you see all
variables and their sources.
I am fine with the pure non-destructuring version or the [value, done] = [...] version

Kind regards / Saludos / Mit freundlichen Grüßen
Wolfram Kriesing, uxebu Co-Founder

kriesing@uxebu.com, mobile: +49 174 300 4595

Learn ES6, one kata a day http://es6katas.org

uxebu GmbH
Goethestraße 28
86161 Augsburg

Amtsgericht Augsburg, Handelsregister HRB 28613
Directors: W. Kriesing, T. v. Klipstein

On Mon, Jul 20, 2015 at 3:10 PM, Tobi Reif notifications@github.com wrote:

If you don't want to require any knowledge of destructuring (neither
object- nor array-based desctructuring), you'd have to get rid of the
current array-based destructuring assignment as well, I think.

From current code

let thisStep = generator;
let [value, done] = [thisStep.value, thisStep.done];

to

let thisStep = generator;
let value = thisStep.value;
let done = thisStep.done;

I could create a PR.

Thanks for the katas!


Reply to this email directly or view it on GitHub
#12 (comment).

@JonathanPrince
Copy link
Contributor

it makes sense to limit the number of new language features in each kata, however using fairly basic ES6 features such as a little destructuring in more advanced katas seems reasonable.
As far as this case is concerned I think its ok as it is, but assigning the two variables separately would also be fine.

@tobireif
Copy link
Contributor Author

I'd also be fine with either of the remaining two options. The decision's up to you Wolfram :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants