Loading collection data...
Collections are a way for you to organize kata so that you can create your own training routines. Every collection you create is public and automatically sharable with other warriors. After you have added a few kata to a collection you and others can train on the kata contained within the collection.
Get started now by creating a new collection.
This comment is hidden because it contains spoiler information about the solution
Resolved with update fork to Node 18
It's an error to me simply because authors/translators should never be rolling their own
shuffle
/random
when we have lodash available for JS, I'll change it.@hobovsky You make a good point, and I don't disagree, but I think the bigger issue is that katas, much like Science experiments, should have as much of a controlled environment as possible to ensure the user can focus on the variable of solving the problem and not figuring out an error unrelated to the problem. Not that this isn't a good lesson for the user to learn, it's just not the focus of the kata and should be avoided. If part of the training was ensuring the user wasn't using
for...in
for this reason then yes, this would absolutely be user error.Also, the kata should be held to a higher standard than the user's code, so the
Array.prototype.shuffle
addition is an error with the kata (IMO).There is no reason for the "for in" not to work here, even if other constructs are available. This is a kata issue, not a user issue.
Yes, extending built-in types should be avoided, but let me ask again: isnt it that a programmer who decided to use
for...in
does things incorrectly, because thefor...in
simply does not work in a way which allows to iterate over arrays (or anything else) reliably? By using potentially misbehaving features, user brings the problem on themselves - especially in face of the fact that there are easily available replacements which work correctly (likefor...of
).I still think that using a wrong language construct (i.e.
for...in
) is solver's error, unless, for some reason,for..in
(i.e. "iterate over all existing keys") would be the solution for this problem, but I don't think it is?You are correct, as stated in the Stackoverflow link.
for..in
should be avoided for this reason as should adding properties to built-in Object prototypes.I agree with hobovsky here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...in#array_iteration_and_for...in
I might be not a JS person, but I think that
for...in
should not be used for exactly this reason, and should be replaced withfor...of
?I think it's not a kata issue, but a misuse of a language feature? Unless, for some reason, you need to iterate exactly over indices and not actual values? Is this a case in this kata?
[javascript] 'shuffle' exposed as index on arrays.
For users having this issue:
The quick fix for this in your code is to change your
for..in
loops to this:For Kata Author:
In test cases, this is getting added to the
Array
object's prototype:Array.prototype.shuffle=function(){...}
As a result,
'shuffle'
gets added as an "index" infor..in
loops.Example:
This will inadvertently cause an error in a user's solution who are using a
for..in
loop in their solution.In general, it's best to avoid modifying the prototype of built-in objects. Instead just create a standalone shuffle helper function, or if you absolutely need to add it to the Array prototype, use the
Object.defineProperty
method:Relevant Stackoverflow: https://stackoverflow.com/a/948379/354317
Also, in the instructions here:
This reader exposes only one method : getChunk()
Returns the following fragment of text from the file it is reading
Returns a string of random size
Returns at least one char
Returns an empty string when finished
I would just get rid of "Returns at least one char" Maybe just have this:
This reader exposes only one method : getChunk()
Returns a fragment of text from the file it is reading as a string.
The fragment can be any length, including
0
(empty string).If the fragment is an empty string, the reader has reached the end of the file, and will only return an empty string on subsequent calls.
I think I understand why it says it will return at least one char, but I think it means to say, "if doesn't return an empty string, the string will be a minimum of 1 char" which is kind of redundant info and confusing.
but what about this test case:
Empty string is a special case which doesn't really make sense in a reader.
But anyhow, that would means nothing, thus :
i do agree this is in conflict with the "Returns at least one char" instruction
(i don't remember if this was in the description i specified 9 years ago or if someone added that line since then)
but defensive programming is always recommended and those values are easily testable
as soon as there is one char, you get a line and a word
but there is a special weird case, what if the text as chars, but it only contains spaces ?
...
there is no word!
From the instructions:
In the instructions, it states that the
getChunk
function exposed by the reader "Returns at least one char".One of the test cases, the first chunk received by the parse method is an empty string (so the entire string it was reading must be an empty string), thus making the charCount, wordCount, and lineCount expected to be 0.
Even if it was allowed that the first string received by
getChunk
was an empty string, there are other tests where there are empty lines that count as a line, so wouldn't an empty string test case still have alineCount
of 1?Example:
and if that's the case, then shouldn't this be true:
Unfortunately that's how CW test runner is written, so I can't overwrite it ;-)
Loading more items...