7 kyu
Odd or Even? Determine that!
268 of 2,241ngntrgduc
Loading description...
Mathematics
Fundamentals
View
This comment has been reported as {{ abuseKindText }}.
Show
This comment has been hidden. You can view it now .
This comment can not be viewed.
- |
- Reply
- Edit
- View Solution
- Expand 1 Reply Expand {{ comments?.length }} replies
- Collapse
- Spoiler
- Remove
- Remove comment & replies
- Report
{{ fetchSolutionsError }}
-
-
Your rendered github-flavored markdown will appear here.
-
Label this discussion...
-
No Label
Keep the comment unlabeled if none of the below applies.
-
Issue
Use the issue label when reporting problems with the kata.
Be sure to explain the problem clearly and include the steps to reproduce. -
Suggestion
Use the suggestion label if you have feedback on how this kata can be improved.
-
Question
Use the question label if you have questions and/or need help solving the kata.
Don't forget to mention the language you're using, and mark as having spoiler if you include your solution.
-
No Label
- Cancel
Commenting is not allowed on this discussion
You cannot view this solution
There is no solution to show
Please sign in or sign up to leave a comment.
This comment has been hidden.
As simple as that.
I don't think it is necessary.
COBOL translation.
Approved.
Shell tests are wrong: https://www.codewars.com/kata/reviews/61dd812be46c3e00018b9402/groups/61e3373c9a04f80001a699af.
Just changed the number of input range... The rest should be better addressed to the Shell translator!
@FArekkusu Have tried to edit the tests but could not publish because the reference solution is also incorrect! I could fix the reference solution and then update the tests! Is it ok or the author translator should do it?
Fixed
though old incorrect solutions are still not being invalidated. Maybe it needs some time to invalidate them...The issue with performance requirements is still not fixed.
C, NASM and Shell are
Fixed
There're no performance requirements in C++ whatsoever.
This correct (and simple) solution does not pass tests in C++. Do you still claim that there are no performance requirements in C++? You need to be lucky with compiler optimizations (or know about possible optimizations) to pass all tests with an inefficient solution.
Okay, the C++ tests do time out for some solutions, but what does this really change? The problem simply narrowed down from "this kata has no performance tests" to "it is possible to pass the performance tests having implemented a bad algorithm" - neither of these should be true. I can see your point but I don't find this "well, people usually write
for
loops this way, and not this way, so they're unlikely to encounter this loophole" argument convincing enough to ignore the problem.Please answer the question: Are there any performance requirements in C++? My answer is "Yes". So this issue is not valid.
No, if inefficient code is not guaranteed to fail, regardless of the reason for that, then you cannot say that there are performance requirements, or at least that they're properly enforced. You may consider this issue not valid since there are indeed "performance tests" which are supposed to check the code's performance, but this does not invalidate the point: the C++ version does not fit the criterias set by other languages (or the other languages do not fit the criterias set by the C++ version (thanks @uniapi for deciding in author's place what this kata should be)) - I can just as well reraise this issue with this particular wording if you like it more.
My point is that all your solutions are efficient. C++ is a high level language. All operations are abstract there. The compiler is smart enough to replace some loops with closed-form fomulas. But it is essentially the same as writing down these formulas yourself. That's how I see this problem. The situtation is similar to Ruby:
(1..10000000000000).sum
is computed efficiently. But one may argue that it isO(n)
(because(1..100000000).sum { |i| i }
should be equivalent to this).But I agree that the performance requirements do not add much to this kata. I would even say that there should be no real performance requirements for low level kata: It is always educational to solve a kata with an inefficient algorithm and then see
O(1)
solutions. My suggestion: close this issue and open a suggestion to remove performance requirements since they do not make this kata more interesting.The initial input range was N ( 1 <= N <= 100 )). This is no longer the case. When did this kata suddenly become about performance (in beta, or after approval)? Also, if author wishes to have performance criteria, state them in the description and tag the kata appropriately. But I hope the author will follow Monadius's advise and remove all performance criteria.
Which is exactly why you should be careful when adding performance requirements in languages which use an aggresive optimizing compiler. It is one thing when the compiler simply makes your code faster due to cutting down on unneeded operations, vectorizing calculations etc. and a completely different thing when it solves the whole kata for you.
Except with the performance requirements involved this whole task is about finding the formula yourself.
To be fair, there's a well known formula for calculating the sum of all integers between
a
andb
, and sinceRange
represents exactly such a thing, it's not surprising thatRange.sum
uses the formula to calculate the sum efficiently. I'd consider this a language feature (a feature of this particular class, to be exact), compared to how the C++ compilers apply numerous arbitrary changes to the code to the point where you yourself have no idea what the resulting assembly code does and why.The author created this kata with
N є [1; 1000]
input range. All 5 translations (Python, C, NASM, JavaScript, Haskell) which were made before the kata approval had performance requirements, just because translators wanted so. As soon as the author started approving those, one of the translators changed the requirements in the original version, so that the kata would become compliant with his work, and not vice versa.@FArekkusu Sorry that adapted C++ to be in the same range as in C and NASM on your request! But really had no intentions to decide in author's place though i do not deny the fact that i did think it would be better to increase the input range! And sure the best way was to ask the author about that change.
So let the author decide... @dfhwze has suggested a good alternative to choose:
@sanshironagato, please, take an action to close this issue forever...
Peace be upon all of you!
Okay, thanks for all of the suggestions. I will remove all performance criteria by decrease the input range to
1 <= N <= 1000
of all languages. The author ofC, Haskell, NASM, JavaScript, Shell
please help me to change the input range of this Kata. Thank you.Resolve issues after they're actually fixed...
Oops, sorry about that :<.
The performance requirements are vastly different across the languages.
Please specify that Python uses
1e8
instead of something on the order of2^64
.You are unnecessarily making it a guessing game again.
I've already linked the discussion explaining what's wrong with this kata's performance requirements. Please, consider learning to read instead of resolving issues based on random bullshit reasoning.
This comment has been hidden.
Please see the Troubleshooting Guide in Docs -> Training.
Ask yourself why you are not even passing the sample tests. Read the description carefully.
Also, know there will be tests with very large numbers. An
O(n)
solution won't cut it.Tests are successful, but when I click "Attempt" I got this:
Test Results: Execution Timed Out STDERR Execution Timed Out (12000 ms)
I don't see another way to code this programm. HELP!
PYTHON
It seem this isn't a issue for the Python translation. Think again, may be your solution is wrong :v. Good luck.
And what's the point of this note?
it's because of a misuse of language conditional blocks (the note section contains information for some languages, like JS)
I have change the description. Does it good now ?
bash translation
Please review.
The performance requirements are vastly different across the languages.
It would really help if you were a bit more specific.
100 * 0 .. 2^64
( I saw no problem to include0
)100 * 0 .. 2^50
(MaxInt
is2^53
, but50
,53
or64
doesn't really matter )100 * 1 .. INTMAX
( probably2^64
)I really can't even read NASM and C.
But I don't see the big problem there.
It would really help if you read the discussion I had with the author where I clearly explained that this kata shouldn't have any performance requirements at all.
I read it. I still don't see what the actual problem is, or how performance requirements are "vastly different."
Your opinion that
O(n)
testing should succeed is nothing more than your opinion; requiring anO(1)
solution is not unreasonable.@JohanWiltink
0 ... INT_MAX
Also see no problem here!
@FArekkusu what problem here ?
Haskell translation
Approving will not affect the Python translation, because this translation does not change the description.
Wording in Kata description:
Should be "is always" instead of "are alway"
This ( and more ) is fixed in the proposed JavaScript translation.
Please don't edit the description in the kata editor now, or the JS translation will need forking because of merge conflict.
There is no guessing involved. I would suggest to change the title to either "Odd or Even?" ( which might already exist ) or something like "Odd or even? Determine that!", or "What's the parity?" Actually, I like the parity one best.
I do also think that
guessing
is not very appropriate! And also suggest one more variant:Odd or Even? Compute it!
Thanks for the suggestions, I will change the title to
Odd or Even? Determine that!
JavaScript translation
Note that this translation changes the description! Make sure you agree with what you are approving.
This translation keeps existing languages intact, but will cause merge conflicts for other pending translations.
May I ask you if I approve this
JavaScript translation
, then the Python description would adapt to theJavaScript description
to be approved with no merge conflict ?If you
then that fork will then be approvable without merge conflicts.
Merge conflicts are the bane of kata translating. With every fork, you have to take care that previous description changes are not reverted, and there can be a lot of forks.
ETA: any time the description is edited directly in the kata editor, any translations that have been created or forked before the moment of that editing will have a merge conflict and will need to be forked and have their description fixed. ( did I mention how much I hate hate hate merge conflicts? :/ )
Woah, thank you for the explanation. I have understand this. I'm also hate the merge conflicts, too :>
The input range is much bigger in C and NASM than in C++.
Adapted
C++ to be in the same range as in C and NASM!The user can still modify the input. Why was this issue resolved without being fixed again?
@sanshironagato: This solution modifies the input and passes all tests. It should be not valid: Compute the expected value before
Assert::That
in random tests.Fixed
@FArekkusu Greatrandom
header!Thank you guys.
The input type should be
unsigned
.Fixed.
The
std::uniform_int_distribution
should probably useunsigned
as the type argument too. It doesn't affect anything here but it's clearer this way that you're actually generating unsigned integers.Fixed.
Please, follow the Unnamed's suggestion and use an
enum
/enum class
instead of strings.Sorry for my lateness :<. Does it good now ?
The enum should be preloaded.
Does it good now ?
I think it's good enough.
It should be
Either
or something likeIndeterminate
, but notBoth
.Fixed.
The description should be formatted properly by using code blocks
The function name should be ommited too to prevent description conflict when more translations are approved in the future
Fixed.
Function name should be removed too and changed to
input: 1
etc. to prevent approval conflict ltr onMissing
#include <string>
headers in initial solution and random test sectionFixed.
The result probably should be an
enum
instead of magic numbers.Oke, I have changed the result.
This issue was not fixed.
@author: perhaps you should let the users that publish an issue resolve issues instead
I don't know how to fix it ;-;
This comment has been hidden.
Any why are you approving low-quality translations instead of pointing out the problems and making the translator fix everything? Are you going to keep fixing the same mistakes forever while the translator keeps making them again and again?
The C++ version of the kata you linked is not even properly fixed - you only corrected the 2 most obvious mistakes, and called it a job...
I didn't realize I missed that part cuz I'm not familiar with C++ enough, but I've fixed those errors after realizing my mistake ltr on. Also, I don't consider not using
unsigned
or looping RNG as an issue (since some katas also do that). So, don't put the blame on mee as I'm also trying to contribute ~~I provided the link just to provide a reference for the author to fix this issue and only this...
PS: I fixed the issue in the other kata, maybe you can take it over next time since you have much experience in C++ alr as to my knowledge instead of just questioning people's efforts in contributing... -_-
You've fixed only one issue, the other is still present.
Take over what?
in taking over fixing katas that you've found prone to errors
Why should I take over anything? All the CW activity which involves reviewing/fixing other people's katas is entirely voluntary. The fact that you're suggesting me to go around fixing all the problematic translations which you approve without thinking is straight-up bullshit - no, thanks.
This was done for a laugh, but I guess it doesn't much matter that the input range is what it is?
ye :v
Fixture is missing the
include
statements.include
what ? :vThe user can modify the input.
Hmm, what do you mean ?
The easiest way to combat this is to compute
expected
value first.Hmm, I have changed a Sample tests.
This isn't about sample tests, it's about random tests. I still don't see any visible changes.
Reference solution should be called first.
Okay, I fixed it.
The reference solution should be inside a
describe
block.Fixed.
This doesn't help much on its own against
#define
. It should be inside anIt
; or it can have a random name that doesn't leak through potential error messages; or anything else than achieves the same effect.random
header functionality should be used instead of plainrand()
. Also, it's not even seeded.Fixed.
Considering how small the input range is, all possible values should be tested. Although, I'd say getting rid of the
[1; 100]
range limitation and adding proper random tests would be a better option.I have ignored the limitation of N and added some random tests.
small issues:
Thanks for the suggestion, I fixed it.
You should add random tests.
Okay, I have added some random tests.