Variable Length Quantity

[jonasbb]

I tried to solve the variable length quantity exercise, because I quite like the encoding scheme. Unfortunately, there are no other implementations of this problem yet, so I am looking for feedback for the function names and test design.

If you know a better way to store bytes back to front into a Vec I would be glad to know this, because allocating a vector and then reversing it seems a bit weird.

Values larger than 0x0FFFFFFF could also be tested in to_bytes. The from_bytes is lacking test for error conditions, like numbers which cannot be represented in u32 and incomplete byte sequences.

[IanWhitney]

Thanks!

New exercises are awesome. I think the normal first step for a new exercise is to add the markdown, yaml and json files to x-common https://github.com/exercism/x-common

That's where I'd normally expect conversations about test design. And other track maintainers are more likely to see conversations in x-common.

Nothing about the Rust implementation jumps out at me. But I've only given it a quick glance.

[jonasbb]

md file and yml file are already present in x-common. It is just that no other language has yet implemented it.

This huge JSON file also contains which other languages implemented a exercise, but no one did for variable lenght.
http://x.exercism.io/v3/tracks/rust/todo

[IanWhitney]

Ok. We can make a first attempt at a test suite here and then extract it to a common json file.

[IanWhitney]

Should this test go first? It seems like it's the simplest to pass, and I usually put the simplest-to-pass tests first.

[jonasbb]

It's an edge case in my solution, where the general does not work. So I did not put it first.

[jonasbb]

Due to the lack of other test cases to compare I took all the examples from the md file and made an assert for each of it. The examples include the smallest, a random, and the largest number representable in x bytes (1 to 4). This might be more than necessary, but does not seem unreasonable to me.

You can definitely pass the single byte cases without passing the multi byte ones. However, I do not think there is much different between the multibyte test case, so that it's likely to pass them all at once. At least if you try to implement a generic solution instead of maybe a more optimal one, which has hard-coded path for each length.

Both identity tests should be redundant too with respect to all of the above. The only new thing they are testing are large vectors (again, general solution). The difference between them is the source of numbers, first 10000 vs pseudo random.

[IanWhitney]

I've not found large vector tests to be very useful. We have some in the sublist exercise and I turned them off when I worked on that problem. All they showed me was that my solution wasn't performant, not that it was invalid.

I do like the idea of random vector tests as an ersatz form of generative testing. I just don't think the number of random elements needs to be large.

Basing the tests off of the Readme makes sense. I hadn't noticed that you'd done that.

In short, what I think we need to do before merging:

• Remove the large vectors
• Have 1 test that uses a small collection of random inputs

[IanWhitney]

Looking a lot better!

I don't think we need to make this the last problem in the track. Looking at the examples, it seems like students will need to use:

• Encoding
• Slices
• Vector
• Bitwise operators
• Result

When I look at other problems it seems like it would fit well after Allergies (which also uses bitwise). I don't know of a problem that features slices, but students may have come across them by this point. If not, this problem would be a nice introduction to as_slice.

@kytrinyx, would a file named example2.rs be copied to the student's machine when they fetch this exercise? I'm not sure how that works.

[jonasbb]

Hi, thanks for the comment.
I merged example.rs with example2.rs to avoid possible problems. I also changed the order in the config file. I just put it in somewhere without thinking much about it.

[IanWhitney]

Great! I think this is looking good. I'll let it sit for a bit and try to get feedback from @petertseng and others. Barring problems I intend to merge this Friday, July 15.

[petertseng]

My only question is do we want to be specific about the messages in the Err or just check that they are is_err? Advantages to both.

I think everything else that I would say has been said

[jonasbb]

It seems to be more common to only check for is_err() instead of a specific string. Notable exceptions are Forth and Circular Buffer, but they provide Error enums for different cases. This would leave Hamming to be the sole exception.

[IanWhitney]

@jonasbb Do you want to rebase this down to a commit or should I do the Squash & Commit?

[kytrinyx]

Quick note: it looks like this got rebased (we don't get notifications when new code is pushed up, so @IanWhitney may not have seen this).

[kytrinyx]

would a file named example2.rs be copied to the student's machine when they fetch this exercise

Nope, anything with example in the path will be ignored (case insensitive regex).

[IanWhitney]

Problem number 36! Thanks very much @jonasbb! 🍰