6 kyu
How Many are Divisible by x
Loading description...
Performance
Algorithms
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.
Nice and mathy kata to learn some theory with.
Test cases in Python would be better if they were factorized a bit more. Currently, there are many repetitions. It's easy to create a generic function
dotest
that does the job for all tests, fixed and random. That would lighten the tests page and would be much more readable. Something like:JS Translation Also to bump the kata (for Approval, maybe?)
Edit: Actually no, a complete rework of the description
ignore
ignore again
The issue below is not fixed. The reference solution still returns
1
form = 12
andx = "2 and 4"
.the real answer should be 3 right?
Yes:
4
,8
, and12
are divisible by2
and4
.Shoudln't be "How many are divisible by x"? Non native english speaker here, but that sounds better.
oh true
This comment has been hidden.
will look into it
.
I don't understand the task. Suppose
m = 12
andx = "2 and 4"
. The reference solution returns1
for this test case. But there are3
numbers in the range (4, 8, 12
) which are divisible by2
and4
.yeah i forgot about that, i have to fix that
thanks for reminding
just fixed
This comment has been hidden.
Actually, the second args should be a string, making it a bit more challenging I suppose
@Insisted it is not about the users solution, it is about the reference solution, which generates results for tests.
This comment has been hidden.
hmmm will look into that
suggestion complete
This comment has been hidden.
the solution first given was very very bad because there was a lot of new featured added and not much of the code was edited making the code spagetti
removed the repeated code
What is the point of testing
m < 0
?Primarily as edge test cases
@Insisted this is tested literally 3 times throught all tests and doesn't make the task harder/easier at all.
Well that, IDK about that
point of m<0 is to check if the range is negative or not, think of it as an additional feature than makeing the kata look harder/easier
Solution setup, sample test cases and test cases are not formatted according to PEP8
are you sure? Did you reset the trainer? On my side, everything looks ok
This comment has been hidden.
who cares about pep8 tho i still have to follow it
follows pep8 as far as i can see now
But it still doesn't........
i used your website idk what is left to do
@mikChan:
Seriously...!? No, that's definitely not an issue. Trying to enforce such a kind of thing will just turn the beta process into riot. ;)
:cough: ... ;p
again:
=> check, double check what changes you introduce.
.
no...
please, don't be so eager to close the issues.
sry github habit
well both are given in the random tests
but the choice between them isn't random.
but for the same range and same x integer there are two different test each for "and" and "or"
And yet, it's not random. Just do the change.
It still makes expected answers unpredictable, they cannot be counted, or hardcoded, or derived easily one from another. You think it is not enough?
made it random
.
...why did you change the function name? Moreover, why did you use
camelCase
for it? In python, you have to usesnake_case
.camelCase is just an old habit of the old lang i used to use
discord #fixing channel
I won't read 100 messages of crossed discussions. What's your question?
he said that the kata was not related to probability anymore and rather it became more of a "tell number of x divisible"
This has nothing to do with the actual problem here (I'm talking casing, not "meaning of the name")
i think everything is snake cased now
marking as solved as i cant seem to find any camelCase function
Sample tests for negative range should also be added
solved
the random tests aren't properly random: you need to generate the string argument randomly too. => random numbers, and random choice for or/and too.
oki
solved
no, no, and no again... See the original sentence:
=> reduce to 200 random tests, and go HUGE on m.
how huge can m go exactly? as i read once it was 2**32 but i think there is more than that for limit
200 tests with random m up to 1e9 will be far more than enough. No need to go higher (that makes translations more tedious in languages that do not support arbitrary integer precision). The point is to forbid any iterative solution, not to find a value/setup that enforce to use up to some seconds to execute with the intended solution.
solved
Since expected solutions are expected to be O(1), you can make
m
generally as large as you like (almost ;) ) You might want to consider potential limits for other languages to make translations more consistent, like2**53
for JS, 64 bit for C or C++, 63 bits for Java and C#, etc. Maybe there are some languages where reasonable max would be2**31
.It's up to you if you want to keep max
m
for Python in sync with other languages or not, but I think it would be generally better to makem
large and have 100-200 tests, rather than 5k small tests.see suggestion below (opening as issue to avoid accidental approval)
oki
and
operatorm
HUGE (or, IDK, 2^32 or 2^31 to make translations easier) and eventually reduce amount of tests.Agreed.
(and points 1 & 2 are actually an issue)
Cool kata!
Example in description is wrong, m=1000 x="2 and 9" is 55 and not 556 as you wrote :) And make somehow clearer that you want the amount and not the probability as return value
Cheers
thanks! will look into the issue
solved
Hi,
Looks to me like the second argument should be a list or a set of integers, not a string.
Cheers
Where to put the
and
/or
then? ;-)no its better with string.
cheers
oh, I missed the "and" example... x)
Would be nicer if you put a message on what is the
x
and alsom
when a test is failingIn almost all cases the user should log the input themselves, since the user should be able to choose if they want to log. Test fixture logging it for user is stripping the user the ability to not log certain values (which will clutter if output if the user wants to log something else.)
It's a suggestion anyway, so new user won't be confused when it is published out of beta ¯_(ツ)_/¯
I think one should only be qualified as a "new user" if they know how to use
print
;-)Well, you are not wrong about that, still I will leave it as a
suggestion
done!
The new non-coprime test expects the wrong result.
looks fine to me ¯\(ツ)/¯
ah nvm
removed the whole co-prime concept.
One fixed test has
m = 0
, which is a bad input (description saysm defines the max range starting from 1
, som
should be at least1
).Duplicate issue.
1000 random tests is not "few random tests", not to mention creating a
it
block per random test will absolutely destroy user's browser performance. They should really be in a singleit
block.will fix that now thanks
Please resolve the issue after they have been fixed, rather than before ;-)
:O
Is the problem with browser performance fixed?
Honestly asking, because i thought that after introduction of lazy loading of test results, the panel should work better now? Didnt bother to check exactly though.
ya fixed, made the it block into one
You did not mention anything about m being
0
as an edge testupdated desc
Rounding to 3 decimal places is a bad idea: numbers ending with
.xxx5
will have around 55% chance of rounding downwards instead of upwards.Either use
Fraction
, or better, return the numerator sincem
is known anyway.i dont think there is any rounding going on in this kata
That'd be even worse; for a lot of big enough integers
a/m + b/m
does not equal(a+b) / m
. Floating point error will be everywhere instead of just at the rounding boundary.should i maybe make it a probability percentage then with xx.xx%
You should probably not use assert_equals since it's won't let anything pass when it has even a 0.00001 value difference
Why not just return the count without divided by
m
? Integers are always exact.yeah i was thinking the same but probability is always between 0 and 1 so maybe i have to change the desc a bit
Hey, Voile, this issue has been raised
implemented this, needs checking. Voile check if the test cases works fine now else everything solved :)
It's not stated in the description that the numbers will be coprime, but in all the test cases (except for where they're equal) this is true. I'd recommend either including this in the description or adding tests with non-coprime values.
they are not supposed to be co prime, i will add some non-coprime values then thanks