Ad
  • Custom User Avatar

    I wrote a lengthy reply on the main discourse, please refer to that so I don't need to address the same issues twice.

    As for a new fork that actually improves the translation regarding assertions and randomness: yes, absolutely! If you manage to implement proper random tests, that would be a huge improvement as well.

    For assertions, please refer to this reference kata to see how to create useful, properly formatted assertion messages.

    For the random tests: there are currently translations pending in Python and Scala (and perhaps others) that implement random tests, although I have not verified whether they do so correctly or succesfully. Take a look if this helps you.

    Final note: if you do end up publishing a new translation, make sure to leave a suggestion linking to it on the kata's discourse, otherwise it is hard to discover.

  • Custom User Avatar

    but a vec of vecs has to follow two pointer inderections.

    Yes, but as I mentioned, the performance overhead is negligible in this particular case. There are very few kata where the cost of pointer indirection or cache misses would actually make a noticable difference in solvability.

    at the very least you should use a single 1d vec and do some arithmetic to index it properly.

    This would break the spec, and is unneccessary complexity in a world where nested arrays/vectors exist and come at virtually no cost (see above).

    yeah, technically the test cases as currently written call for this, but i don't think this should be.

    You must be new here ;)

    While the points you mention are valid, and in a perfect world where everything was strictly idiomatic this kind of thinking would totally be forced into consideration, very few kata actually conform to it. There are lots of reasons for this: author laziness, author ignorance, trying to force a square peg (translation) through a round hole (task authored in another language), avoidance of diverging from existing translations, etc.

    Personally, I detest kata where validation is forced on the user for no other reason than the author thought it would be more interesting (It's not. Never is.). However, relating kata to production code is simply a fool's errand; few of the assumptions you could make about input, structure, etc. in production apply here, where the author is free to make up the wildest formats, requirements, edge cases, etc.

    If idiomatic code is important to you, I'd suggest directing your efforts at new translations of kata where Rust is not yet an available language. As the translation author, you are free to be as idiomatic as you'd like, as long as this does not entail diverging from specs or uniformity with existing language translations. This would be greatly appreciated, in fact!

    As for this kata, it's just not worth the invalidations. Mind you, as I said elsehwere, there are cases where we allow a fork to invalidation dozens or even hundreds of solutions, but usually that's when an actual error is detected, i.e. correctness is at stake. Unidiomatic code isn't really incorrect, merely slightly frustrating for those who know better. Hence my rejection of your fork. I hope you understand.

    Cheers.

  • Custom User Avatar

    no, my proposed change means "don't use a pointer of pointers". I agree heap vs stack is irrelevant, but a vec of vecs has to follow two pointer inderections. this absolutely can have extra overhead. at the very least you should use a single 1d vec and do some arithmetic to index it properly. The only time i use a vec of vecs is when i actually need an array over arrays of many different sizes. yeah, technically the test cases as currently written call for this, but i don't think this should be. and though i haven't checked, your comment seems to imply not all languages take this approach. But more importantly, defining types concretely like this is less error prone. the size of a sudoku board doesn't change, and it's always square, so defining your type that way is a great help. if you want to handle missing values, then i would do it like this:

    Sudoku<const N: usize>{
        data: [[Option<u32>; N]; N]
    }
    

    this actually makes sense, as a sudoku can't be oddly sized, but it can have missing numbers.

    if this "loose typing" approach can be avoided, then it should be. but that's just my two cents.

    edit: i was wrong, just checked and it seems all the language translations have oddly sized inputs. this is an odd choice to me, but it's at least consistent.

    to see more of what I'm talking about, look up "parse, dont validate".

  • Custom User Avatar

    totally fair, I can accept that invalidating current solutions is a no go.

    reguarding bad assert messages and random board sizes, the assert macros are the same as the current version, and random board sizes are also not used. In fact, when generating random solutions, the current version only creates sizes of 4x4 and 9x9, despite comments claiming to create 3x3 (which is an invalid size), 9x9, and 16x16.
    would you like me to submit a different fork that keeps the solution the same and addresses these already existing problems?

    reguarding a vec of vecs being "completely fine", I personally disagree. the type system should be leveraged as much as possible to make invalid states unrepresentable. if you want me to handle garbage input, then give me a string to parse. yeah it works, but it is more like the way things are done in dynamically typed languages. On balance, however, i get that breaking current solutions is a problem.

  • Custom User Avatar

    As mentioned in the discourse, this change isn't sufficiently useful to justify invalidating current solutions.

    That said, there are other issues with this translation (bad assertion messages being one of them). Also, I just realized that using const generics means that there can be no random board sizes, since the rng is not const. That's actually a major regression that disqualifies const generics for this (and probably many other) kata.

    A vec of vecs is completely fine, so I'm going to reject this translation.

  • Custom User Avatar

    There is no way to do that without invalidating all current solutions. While it is sometimes justified to do so, I wouldn't claim this to be the case here; your proposed change effectively just means "allocate on stack instead of heap", which would have virtually no measurable effect on the actual performance or the required solution. Further, some translations (whether justifiably or not) use jagged matrices (in the sense that some rows are shorter than others). This wouldn't be possible with const sizes.

  • Custom User Avatar

    This is a more idomatic way to handle 2d arrays of data.
    a vec of vecs is generally a bad representaton, and with the stabilization of const generics in rust this better method can be used.

    there is a downside, however: current solutions won't work anymore.

  • Custom User Avatar

    rust should use a const generic and 2d array instead of a vec of vecs.
    e.g.

    Sudoku<const N: usize>{
        data: [[u32; N]; N]
    }
    impl<const N: usize> Sudoku<N> {
        fn is_valid(&self)-> bool{}
    }