Ad
  • Custom User Avatar

    Not only is 2kB an insignificant amount of memory to use for this, but also you already have that memory available. On most systems, the default stack size for the main thread is 2MB. Unless the rest of your program is using 99.9% of its available stack, it costs almost nothing to dedicate 2kB of stack to make this array, and pop it from the stack after this function finishes.

    As for heap allocation, I'm not very familiar with Rust's allocator, but it's quite possible that creating a hash map for this will in some cases be forced to take 4kB, since most operating systems think about memory in 4kB chunks.

  • Custom User Avatar

    This is no real assembly, just follow the description: why should we care about the registers size? And without any precision it is clear that you are not expected to operate on unsigned integers. About the rewording, this is at most a suggestion, not an issue.

  • Custom User Avatar

    The array is indisputably the best solution. What's the better alternative, using a hashmap? No allocator code has to be called at all to do this, just decrement the stack pointer and zero the elements.

  • Custom User Avatar
  • Custom User Avatar

    Nop, it was a clever insight to see its a slice of u8 so we can trivially allocate 2kb in the stack. If for some reason you're worried this is too much, we can always reduce to u32 and assert the slice is no bigger than that. However, modern CPUs easily have 32kb available per execution

  • Custom User Avatar
  • Custom User Avatar
  • Custom User Avatar

    Stack allocations are faster than heap allocations

  • Custom User Avatar

    @docgunthrop Thanks!

  • Custom User Avatar

    @kazk: Thanks, the kata has been updated with the code from your most recent fork. Example tests, description, and the initial test setup have also been updated.

    I'd like you to have your future Rust translations reviewed by Rust users (we have few very active ones). You're the author of the kata, so it's not a requirement enforced by the system, but this is one of the main reasons why we have unidiomatic translations.

    Sure I'm fine with that.

    Regarding the inefficient RNG functions, I'm aware of the shortcomings, but it seemed to have very little impact on real-world performance with respect to the test constraints of the kata. I preferred to keep the RNG functions segregated from the non-RNG code and the runtime cost seemed negligible.

    Also, gen_range can take inclusive range x..=y

    Yes, it's already used elsewhere in the exec_rng_tests function. I may have had some non-obvious reason for using the exclusive range in the RNG code but nothing comes to mind ATM.

  • Custom User Avatar

    Thanks, I forked your solution to suggest few things: https://www.codewars.com/kumite/606cd9869bd117ad047aabf3?sel=606cdd479bd117003392c52e

    mod ziggurat {
        pub fn ride_of_fortune(artifact: &[&str], explorers: &[u8]) -> Vec<Option<(u8, u8)>> {
            todo!();
        }
    }
    
    1. match the integer type in explorers and in the returned pairs
    2. using slice because it's unnecessary to pass Vec, and the test doesn't need to clone

    I'd like you to have your future Rust translations reviewed by Rust users (we have few very active ones). You're the author of the kata, so it's not a requirement enforced by the system, but this is one of the main reasons why we have unidiomatic translations.

    I didn't include in the fork, but the following is inefficient:

    fn randbool() -> bool {rand::thread_rng().gen_bool(0.5)}
    fn randint(x:usize,y:usize) -> usize {rand::thread_rng().gen_range(x..y+1)}
    fn randfloat(x:f32,y:f32) -> f32 {rand::thread_rng().gen_range(x..y+1.0)}
    

    These functions creates thread local generator for each call and these are called many times. Also, gen_range can take inclusive range x..=y. I'd inline these, or at least change them to take the generator.

  • Custom User Avatar

    @kazk: Updated the type signatures for the Rust translation as shown below. If you encounter any issues, feel free to let me know.

    mod ziggurat {
    	pub fn ride_of_fortune(artifact: Vec<&str>, explorers: Vec<u32>) -> Vec<Option<(u8,u8)>> {
    		todo!();
    	}
    }
    
  • Custom User Avatar

    not sure how I overlooked that 😅

  • Custom User Avatar

    Yes, as was using [-1,-1] instead of having to deal with Options. It was intended to remove some barrier so the user can focus on the core task. Same reason for C# and Go mentioned earlier.

    Go's type system is not expressive enough, so we don't have a choice. The standard library uses -1 when some index is not found, so [2]int{-1, -1} is not that awkward. C# doesn't have sum types either.

    For Rust, I think it's actually a distraction because it's awkward. We never use -1 like this. I also tried replacing (-1, -1) with None in your solution, and I don't see how Option can be a barrier. It's cleaner with None since there's no need tolet null: (i32, i32) = (-1, -1);, if v != null, vec![(-1,-1); 4], etc.

    If the request to change the expected output still stands, I don't mind making the update.

    Thanks, please update.

    I'd also suggest using a slice (&[T]) instead of Vec<T> (as far as I can see, it's unnecessary to pass Vec<T>), so:

    fn ride_of_fortune(artifact: &[&str], explorers: &[u32]) -> Vec<Option<(u32, u32)>> {
        todo!();
    }
    

    u32 can be u8 (since 6 <= n <= 40) or usize (can be more convenient).

  • Custom User Avatar

    why None is expected in Python, but not in Rust?

    The kata expects a tuple [-1,-1] for C# and Go translations.

    I'm assuming i32 is used just to allow -1.

    Yes, as was using [-1,-1] instead of having to deal with Options. It was intended to remove some barrier so the user can focus on the core task. Same reason for C# and Go mentioned earlier.

    Otherwise, please consider fixing it. I can always ignore authors and change it myself, but I'd rather not do that.

    If the request to change the expected output still stands, I don't mind making the update.

  • Loading more items...