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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Main example for priority queue using dijkstra's algorithm. #15857

Merged
merged 1 commit into from Jul 22, 2014

Conversation

Projects
None yet
5 participants
@treeman
Copy link
Contributor

treeman commented Jul 21, 2014

I wanted to have a slightly larger example compared to the method examples, but I'm unsure how it worked out.

Feedback would nice.

//! #[deriving(Eq, PartialEq)]
//! struct State {
//! cost: uint,
//! pos: uint

This comment has been minimized.

@steveklabnik

steveklabnik Jul 21, 2014

Member

bikeshed alert: wonder if this shouldn't be a full position instead. It's not that much longer.

//! pos: uint
//! }
//!
//! // The priority queue depends on `Ord`

This comment has been minimized.

@steveklabnik

steveklabnik Jul 21, 2014

Member

This should probably have a period at the end.

//!
//! // The priority queue depends on `Ord`
//! // Explicitly implement the trait so the queue becomes a min-heap
//! // instead of a max-heap

This comment has been minimized.

@steveklabnik

steveklabnik Jul 21, 2014

Member

Same here.

//! }
//! }
//!
//! // Currently `PartialOrd` needs to be implemented as well

This comment has been minimized.

@steveklabnik

steveklabnik Jul 21, 2014

Member

Words like 'currently' shouldn't be in the docs. Everything is current. I know it might change in the future, but then this has to change anyway.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jul 21, 2014

I really like this idea.

@treeman

This comment has been minimized.

Copy link
Contributor Author

treeman commented Jul 22, 2014

Great! Thanks for the pointers.

//!
//! // Examine the frontier with lower cost nodes first (min-heap)
//! while !pq.is_empty() {
//! let State { position: u, cost: curr_cost } = pq.pop().unwrap();

This comment has been minimized.

@huonw

huonw Jul 22, 2014

Member

This could be

loop {
    let State { position, cost } = match pq.pop() {
        None => break, // empty
        Some(s) => s
    };

    // ...
}
//! // The graph is represented as an adjecency list where each index,
//! // corresponding to a node value, has a list of outgoing edges.
//! // Chosen for it's efficiency.
//! let graph = vec!(

This comment has been minimized.

@huonw

huonw Jul 22, 2014

Member

These can/should be vec![...] (rather than vec!(...)).

//! if position == goal { return cost };
//!
//! // Important as we may have already found a better way
//! if cost > dist[position] { continue };

This comment has been minimized.

@alexcrichton

alexcrichton Jul 22, 2014

Member

This semicolon (and the one above) could be removed.

This comment has been minimized.

@steveklabnik

steveklabnik Jul 22, 2014

Member

I'd even argue should, it's usually the style

//! fn shortest_path(adj_list: &Vec<Vec<Edge>>, start: uint, goal: uint) -> uint {
//! // dist[node] = current shortest distance from `start` to `node`
//! let mut dist: Vec<uint> = Vec::new();
//! dist.grow(adj_list.len(), &uint::MAX);

This comment has been minimized.

@alexcrichton

alexcrichton Jul 22, 2014

Member

You could combine these two with let mut dist = Vec::from_elem(adj_list.len(), uint::MAX);

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jul 22, 2014

This is a fantastic example, nice job!

This looks good to go with two minor comments and some squashings, thanks!

@treeman

This comment has been minimized.

Copy link
Contributor Author

treeman commented Jul 22, 2014

Cool, thanks!

@bors

This comment has been minimized.

Copy link
Contributor

bors commented on 94500b8 Jul 22, 2014

This comment has been minimized.

Copy link
Contributor

bors replied Jul 22, 2014

merging treeman/rust/doc-dijkstra-example = 94500b8 into auto

This comment has been minimized.

Copy link
Contributor

bors replied Jul 22, 2014

treeman/rust/doc-dijkstra-example = 94500b8 merged ok, testing candidate = 853f53e

This comment has been minimized.

Copy link
Contributor

bors replied Jul 22, 2014

This comment has been minimized.

Copy link
Contributor

bors replied Jul 22, 2014

merging treeman/rust/doc-dijkstra-example = 94500b8 into auto

This comment has been minimized.

Copy link
Contributor

bors replied Jul 22, 2014

treeman/rust/doc-dijkstra-example = 94500b8 merged ok, testing candidate = 2ffccb7

This comment has been minimized.

Copy link
Contributor

bors replied Jul 22, 2014

fast-forwarding master to auto = 2ffccb7

bors added a commit that referenced this pull request Jul 22, 2014

auto merge of #15857 : treeman/rust/doc-dijkstra-example, r=alexcrichton
I wanted to have a slightly larger example compared to the method examples, but I'm unsure how it worked out.

Feedback would nice.

bors added a commit that referenced this pull request Jul 22, 2014

auto merge of #15857 : treeman/rust/doc-dijkstra-example, r=alexcrichton
I wanted to have a slightly larger example compared to the method examples, but I'm unsure how it worked out.

Feedback would nice.

@bors bors closed this Jul 22, 2014

@bors bors merged commit 94500b8 into rust-lang:master Jul 22, 2014

2 checks passed

continuous-integration/travis-ci The Travis CI build passed
Details
default all tests passed

@treeman treeman deleted the treeman:doc-dijkstra-example branch Sep 17, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.