Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Examples [WIP] #10

Merged
merged 2 commits into from Dec 2, 2014
Merged

Examples [WIP] #10

merged 2 commits into from Dec 2, 2014

Conversation

moonglum
Copy link
Contributor

Ok, next pull request coming up!

I've build a buggy version of a quicksort implementation with a quite common bug (neither the smaller_than nor the larger_than part include the "equal" part). QuickCheck will find this bug.

I also added a more descriptive name to the invariant on your reverse example. I hope you like it 馃槈

Work in Progress: I don't know how elegant my implementation is. I tried to keep it to my Haskell implementation, which is only 5 lines. This implementation gets kinda messy with conversions between vectors and ranges. I will ask for feedback in the Rust IRC 馃憤

@@ -0,0 +1,61 @@
// This is a buggy quick sort implementation, QuickSort will find the bug for you
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/QuickSort/QuickCheck

@BurntSushi
Copy link
Owner

@moonglum Hey, so you've picked an interesting time to submit PRs. We're in the middle of a hackathon and @huon has packaged this crate up to be submitted to Rust proper. Here's his fork: https://github.com/huonw/rust/tree/quickcheck

Would you mind submitting your em-dash PR to that fork?

I'm not sure if the examples made it in.

result.push(x.clone());
result.extend(ys.iter().map(|x| x.clone()));
result
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might find the append and append_one methods (on the Vec type) useful here.

@moonglum
Copy link
Contributor Author

I'm actually at that hackathon! 馃槃 I will do that!

@BurntSushi
Copy link
Owner

@moonglum Oh, awesome!

@moonglum
Copy link
Contributor Author

About the other PR: Not sure if that applies any longer as the Markdown will now no longer be parsed by Github, but my the Rust Documentation Tool. I don't know if it has the same bug 馃槈

@moonglum
Copy link
Contributor Author

Thanks for all your feedback, I will work it into a new version. Where will the examples be in the future?

@huonw
Copy link
Contributor

huonw commented May 11, 2014

@moonglum just add them to the doc comment of the main lib.rs file: e.g. add a "# More examples" (or "# Case study: sorting") section at https://github.com/huonw/rust/blob/quickcheck/src/libquickcheck/lib.rs#L373-L374 .

@moonglum
Copy link
Contributor Author

@BurntSushi Thank you so much for your feedback! Helped a lot. What do you think? Do you still see things to be improved?

@huonw Cool! I will do that!

@huonw
Copy link
Contributor

huonw commented May 11, 2014

It seems to be a little tricky to send pull requests to a repository that's not the source of a fork (i.e. if a fork of rust comes from mozilla/rust then others haven't been able to submit PRs to huonw/rust). Feel free to just push the commit to a normal branch and put a link to that here; I'll cherry pick it to put it in rust-lang/rust#14100.

@emberian
Copy link

@BurntSushi can this be merged now?

BurntSushi added a commit that referenced this pull request Dec 2, 2014
@BurntSushi BurntSushi merged commit 4e49c87 into BurntSushi:master Dec 2, 2014
BurntSushi added a commit that referenced this pull request Dec 2, 2014
@BurntSushi
Copy link
Owner

Sorry about the late merge, but better late than never. :-)

@moonglum moonglum deleted the examples branch December 3, 2014 14:41
@moonglum
Copy link
Contributor Author

moonglum commented Dec 3, 2014

Thanks 馃憤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants