Ad
  • Default User Avatar

    just else and one x.count are extra. Much better to call functoin once.

  • Custom User Avatar

    If there are no good ideas ("as is often the case") then it traverses array only once.

  • Default User Avatar

    This always traverses array two full times.

  • Custom User Avatar

    actually, the real problem is rather that the kata is unsolvable in its current state, imo (no completion since you added your last requirements!). I don't think you can keep those requirements and make something interesting of the idea (mostly because if you keep the current version, the only way to make it completable is to provide an almost step by step algo so that one can find the same solution than yours).

  • Default User Avatar

    Plan for modifications:

    1. clarify descroption to make it clearer what the mathematical rules are
    2. change centre of disk to any float value to avoid the possibility of finding all "failure" cases
    3. move make_disk as suggested
  • Custom User Avatar

    It should be inside example_tests() and random_tests(), and the make_disk() in the preloaded should be removed/overwritten at the start of the tests.

    On the other hand, your reference solution is very slow, that's why you can't run more tests. You could use a precalculated lookup-table (or rather, a directory with reduced number of examples) for the tests, e.g.:

    @test.describe('Random Tests')
    def random_tests():
        SOLUTIONS = {28.6: -1, ..., 52.0: 52.0, 52.1: 52.1, ...}
        
        radius = random.choice(SOLUTIONS)     # choose a stored value
        expected = SOLUTIONS[radius]
        disk = make_disk(radius)
        test.assert_equals(find_radius(disk), expected)
    
  • Default User Avatar

    although more random tests wouldn't help in this instance I believe as the edge cases will still be the same ones. Where should make_disk go then? (I'm new to authoring kata so I'm still figuring this stuff out)

  • Custom User Avatar

    This comment is hidden because it contains spoiler information about the solution

  • Default User Avatar

    well spotted - will have a think to see whether I can stop the brute force approach to find edge cases

  • Custom User Avatar

    except that now, your solution is too slow for the current version of the tests: consistently times out in 3.4.3. 11,2-11,5s in python 3.6.0. That's too much.

    • why do you mutliply by 1.0? Seems that all tests are passing either way (and drops the duration to 8,5-9,5s without it)
    • round one time and only one time, not every possible time in the loops (moreover, two times per iteration...)

    I'm still not sure about the consistency of the results if someone doesn't use the exact same approach as yours, tho... 5for now, I'm just exploring a bit your solution).

  • Default User Avatar

    Indeed, thanks for pointing that out – the new solution fixes the case when no solution is found (and indeed there were cases before when the wrong solution was found ). I believe the latest reference solution fixes both issues as I'm taking care of the precision of radius within the code now

    Brute force: guilty as charged. I'll look for a cleaner and neater solution (and hopefully I'll see some better solutions by others posted soon)

    Reference solution now rounds to 1 decimal place.

  • Custom User Avatar

    Mmmh... I misread the code, you don't use sqrt in there. Might be enough, yes. Tho, your way is quite brutal.

    Ah, actually, no, that's not enough: Put that in your solution if not answers: raise and you'll see that your code sometimes go wrong, finding no solutions at all

    Note that your reference solution is rounding with 2 decimals, not one.

  • Default User Avatar

    Thanks for your comment. Do you have an example in which you think the reference solution is incorrect? The requirement states that "if the centre of the pixel is within the disk, then it is represented as a 1, and if the centre of the pixel is outside the disk then its value is 0." This is not an ambiguous condition since the centre of a pixel, say (2, 2) is either within the radius of the circle or outside. This gives a clear one-to-one mapping from radius to array but possibly a many-to-one inverse mapping from array to radius. This requirement means that the solution is dependant of a "specific" precision requirement but that the solution needs to solve the above requirement.

    Possibly the old version could be a separarte easier kata but this version poses a more interesting challenge, I believe. Happy to discuss further of course, hoping to clarify to you and hence in the kata description the actual requirement being made

  • Custom User Avatar

    your reference solution is incorrect with the new requirements:

    • you do not take care of floating point errors
    • there are a lot of ways to approach the precision requirements. You cannot let the user guess. Honestly, without that requirement, the kata was good enough. Now... not anymore.
  • Default User Avatar

    I have updated this beta to include more random test cases and to include the option of indeterminate cases in which the input array does not have sufficient resolution to return a unique radius.

    Comments welcome…

  • Loading more items...