Ad
  • Custom User Avatar

    I'm not sure why the previous fork didn't get approved after so many comments, since everyone's concerns appear to have been adressed. That said, before approving, I took the opportunity to slightly improve it a bit:

    • Changed the parameter type for number_of_sides to u32 (can't have negative number of sides, that would make it an antigon, and we haven't discovered those yet)
    • Made random tests generate radii with a single decimal digit instead of just integers.
    • Cleaned up the custom assertion macro. It now outputs much more useful assertions. I'm not sure why we need it since the calling signature is always identical anyway, but I left it there for future reference I guess.

    I'll leave this here to cook for a while. If there's no issues, I'll then approve myself.

  • Custom User Avatar

    Notes:

    • The description example uses findSummands instead of find_summands
    • test module should only use super::find_summands, not everything (*).
    • Your assertions have messages, which is nice, but they are still incomplete: left and right (mentioned in the default assertion) are meaningless to users who can't see the test.
    • Why are you using #[track_caller]?
    • In the random tests, the assertion that checks the sum mentions "incorrect length" in the message.
    • The assertion for consecutive numbers doesn't actually do that. It just checks that the first is smaller than the second. You could just write w[0]%2==1 && w[1]==w[0]+2.
    • Checking length, sums, and other properties separately is needlessly convoluted. For fixed tests, you know exactly what the resulting slice should look like. For random tests, you can just generate a reference solution (which would actually be faster than iterating over the user solution multiple times like you do here). The chance that a user solution fails only one or two, but passes the rest is virtually nil.
    • I think a few more than just 10 random tests would be in order.

    Rejecting as-is, feel free to fork and update with fixes.

  • Custom User Avatar

    Notes:

    • u64 would probably be a better fit than usize, even if these are technically the same on codewars platform. Good habits and all...
    • A sample test with a large input which causes the default implementation to time-out would be useful.
    • Missing fixed tests in the full test suite. Re-use those from the sample tests.
    • Your random tests are broken: you generate a single random number, then use that same number 1000 times to peform identical tests. You probably meant to move the let n = rng... into the loop.
    • Missing assertion messages in sample tests (yes, they belong everywhere)
    • In Rust, the default assertion message for assert_eq! mentions "left" and "right", assuming the user can read the tests and thus knows which is which. On Codewars, this is not the case. The assertion messages need to explicitly mention what "left" and "right" refer to (e.g. actual and expected)
    • Please don't use super::* in the test module. Only import the items you need from the user's solution code (i.e. use super::find_x), keep your test module hygienic.

    I will reject this translation as it is. Please fork it and apply fixes.

  • Custom User Avatar

    Also please check the most recent fork to see a solution that bypasses all this faff in the first place :)

  • Custom User Avatar

    This fork solves the problem of only running the name test when everything else has passed. It does this by using a single decorated #[test] function, and rolling my own describe and it blocks with custom testing functions.

    Each it still fails early, but successive tests still get to run (unless/until something times out).

    Strictly, this is nesting a describe inside an it (the #[test] decorated function itself emits an <IT::>), but the runner doesn't appear to mind.

  • Custom User Avatar

    Please disregard this atrocity. It's silly and I regret ever suggesting it. I've since published a downstream translation that solves this properly.

  • Custom User Avatar

    The way AtomicU32 is used here doesn't make sense. Using static mut in multiple threads without other synchronization is straight up undefined behavior, and get_mut completely defeats the purpose of atomics in the first place.

    static TESTCHECK: AtomicU32 = ... and fetch_add is probably what was intended. See: https://doc.rust-lang.org/std/sync/atomic/struct.AtomicU32.html#method.fetch_add

  • Custom User Avatar

    While the upstream translation technically works, it really is kinda ugly the way a failed final tests shows up.

    Now, I know this implementation is super hacky, but seems to work. Basically, it suspends the final test thread to give other tests time to run, and repeatedly checks whether the success conditions are met. I am unsure how this interacts with solutions that are slower, though.
    Just an experiment.

  • Custom User Avatar

    Old solutions get invalidated if a change is made to the test suite for the kata (no honor is lost)

  • Custom User Avatar

    If you can remove the requirement safely, I think it's better to do it.
    Note that I don't receive notifications for this discussion so I may miss further elements (I may be less present in next days), as I said the translation looks good.

  • Custom User Avatar

    With a margin of 1e-3 I don't think that'll be a problem. They should work too.

  • Custom User Avatar

    I don't know if there is a risk to break old solutions in already approved languages, then it can be better to maintain it, my idea was at least not to maintain for new languages since it doesn't make much sense (in this case the description would need to be edited). Apart from that the translation looks good.

  • Custom User Avatar

    Well then, I'll remove the rounding then and use a tolerance.

  • Custom User Avatar

    I don't like it, this kind of test makes sense for decimal number, floats are not good for this (what if 3.4499999999... or 3.44500000000000001 ?). I believe the interest of the kata is the geometry stuff, not rounding to three decimals, which is more like 8/7kyu level task and is already done in some other kata.

  • Custom User Avatar

    Could there be 2 kind of tests? Check javascript version, Maybe it's impossible to do it because not all numbers can be represented, but...

  • Loading more items...