Dijkstra's algorithm #428

wow-such-amazing · 2017-04-04T07:09:29Z

No description provided.

wow-such-amazing · 2017-04-12T08:49:58Z

should i change something?

kelvinlauKL · 2017-04-13T20:06:15Z

@crabman448 checking it out now

kelvinlauKL

First of all, great work!

It's great to have one of the most famous algorithms written up for our repo.

The readme file needs a fair bit of work. I've left some more details below, but ideally you should give a mid to high level explanation of the interesting bits of the algorithm. Given the fame of this algorithm, there should be lots to write about.

In addition to the code level comments written below, I've got a few RW style guidelines I want to mention:

omit needless self references. You've got many of those in your implementations
extensions can give your code better readability for protocol conformances

The tool looks/sounds interesting. I won't be reviewing the code that implements it, but I'm looking forward to playing around with it!

Cheers!

kelvinlauKL · 2017-04-13T20:15:40Z

Dijkstra Algorithm/README.md

@@ -0,0 +1,23 @@
+# Dijkstra-algorithm
+WWDC 2017 Scholarship Project
+Created by [Taras Nikulin](https://github.com/crabman448)


Put the credits at the bottom.

kelvinlauKL · 2017-04-13T20:20:05Z

Dijkstra Algorithm/README.md

+
+## About
+Dijkstra's algorithm is used for finding shortest paths from start vertex in graph(with weighted vertices) to all other vertices. 
+More algorithm's description can be found in playground or on wikipedia:


Dijkstra's algorithm is one of the most famous graph algorithms. Instead of giving a link to wikipedia, we'll want a bigger sample of what this algorithm is about.

Some things to talk about:

Why it is so often studied

Real life problems that it solves

A summary of how the algorithm works

kelvinlauKL · 2017-04-13T20:21:14Z

Dijkstra Algorithm/README.md

+More algorithm's description can be found in playground or on wikipedia:
+[Wikipedia](https://en.wikipedia.org/wiki/Dijkstra%27s_algorithm)
+
+To understand how does this algorithm works, I have created **VisualizedDijkstra.playground.** It works in auto and interactive modes. Moreover there are play/pause/stop buttons.


Sounds cool, will check this out.

kelvinlauKL · 2017-04-13T20:23:34Z

Dijkstra Algorithm/README.md

+
+## Screenshots
+
+<img src="Screenshots/screenshot1.jpg" height="400" />


These screenshots don't give me any context on what it's about. If this is just to showcase your visualization tool, 1 screenshot right below the paragraph where you introduce your tool is enough.

kelvinlauKL · 2017-04-13T20:24:26Z

Dijkstra Algorithm/Dijkstra.playground/Contents.swift

@@ -0,0 +1,137 @@
+//: Playground - noun: a place where people can play
+
+import Foundation


Remove unnecessary imports.

kelvinlauKL · 2017-04-13T20:28:17Z

Dijkstra Algorithm/Dijkstra.playground/Contents.swift

+ }
+
+ public static func ==(lhs: Vertex, rhs: Vertex) -> Bool {
+ if lhs.hashValue == rhs.hashValue {


if lhs.hashValue == rhs.hashValue { return true } return false

is equivalent to

return lhs.hashValue == rhs.hashValue

kelvinlauKL · 2017-04-13T20:29:26Z

Dijkstra Algorithm/Dijkstra.playground/Contents.swift

+
+open class Vertex: Hashable, Equatable {
+
+ open var identifier: String!


Why is this an implicitly unwrapped optional (IUO)?

kelvinlauKL · 2017-04-13T20:38:17Z

Dijkstra Algorithm/Dijkstra.playground/Contents.swift

+
+ open var identifier: String!
+ open var neighbors: [(Vertex, Double)] = []
+ open var pathLengthFromStart: Double = Double.infinity


The Double type can be inferred from Double.infinity, no need to do explicit typing.

kelvinlauKL · 2017-04-13T20:38:49Z

Dijkstra Algorithm/Dijkstra.playground/Contents.swift

+ }
+}
+
+public class Dijkstra {


As with Vertex, let's put Dijkstra in it's own file in the Sources directory.

kelvinlauKL · 2017-04-13T20:46:14Z

Dijkstra Algorithm/Dijkstra.playground/Contents.swift

+ startVertex.pathLengthFromStart = 0
+ startVertex.pathVerticesFromStart.append(startVertex)
+ var currentVertex: Vertex! = startVertex
+ while currentVertex != nil {


This method depending on an IUO (implicitly unwrapped optional) makes me nervous.

For starters, instead of using Vertex!, you can have it as Vertex? and achieve the same looping logic:

while let vertex = currentVertex { ... }

Try to refactor this method based on that.

wow-such-amazing · 2017-04-16T09:14:51Z

Thank you for your comments! 👍
I will fix it as soon as possible :)

This reverts commit a153067.

This reverts commit 3f6be33.

This reverts commit a153067.

kelvinlauKL

Left a few more comments on your latest change.

kelvinlauKL · 2017-04-17T19:41:14Z

Dijkstra Algorithm/Dijkstra.playground/Contents.swift

+//: Playground - noun: a place where people can play
+import Foundation
+
+var _vertices: Set<Vertex> = Set()


What's the reason for the _ in front of this variable name?

kelvinlauKL · 2017-04-17T19:49:31Z

Dijkstra Algorithm/README.md

@@ -0,0 +1,45 @@
+# Dijkstra-algorithm
+
+## About this repository


Put the About, Demo Video, and Screenshots section at the bottom of this file.

kelvinlauKL · 2017-04-17T20:00:36Z

Dijkstra Algorithm/README.md

+
+<img src="Images/Dijkstra_Animation.gif" height="250" />
+
+[Wikipedia](https://en.wikipedia.org/wiki/Dijkstra%27s_algorithm)'s explanation:


Please don't just copy from Wikipedia. If it helps, you can quote a small paragraph from it, but most of the writing should be in your own words.

What you've put here is the procedure of Dijkstra's algorithm. Before talking about the procedure, you need to talk about what the algorithm is, and what it will solve. I suggest you take some inspiration from this article: https://github.com/raywenderlich/swift-algorithm-club/tree/master/Merge%20Sort

kelvinlauKL · 2017-04-17T20:01:17Z

Dijkstra Algorithm/README.md

+
+## Dijkstra's algorithm explanation
+
+<img src="Images/Dijkstra_Animation.gif" height="250" />


This diagram needs more context for the reader.

Consider that the reader is new and first opens this article. That person wouldn't and shouldn't know what this diagram is. You'll want to place this diagram in between an explanation to give it context.

wow-such-amazing · 2017-04-17T20:56:13Z

Will improve it in a couple days. Thank you again! : )

kelvinlauKL

Hey @crabman448,

Great improvements overall, but the explanations still need some work! I've left some comments that I feel would greatly improve this article.

For now, let's focus on polishing the first half of your readme. Some things to keep a note of:

grammar
punctuation
spelling mistakes

There are many of these in the readme. Try to fix those as you're making changes.

kelvinlauKL · 2017-05-01T23:54:27Z

Dijkstra Algorithm/README.md

+
+Let's imagine, you want to go to the shop. Your house is A vertex and there are 4 possible stores around your house. How to find the closest one/ones? Luckily, you have graph, that connects your house with all these stores. So, you know what to do :)
+
+## Initialization


Rename this to "Example" instead. Initialization doesn't mean anything to the reader for this section.

kelvinlauKL · 2017-05-01T23:56:55Z

Dijkstra Algorithm/README.md

+
+I have created **VisualizedDijkstra.playground** to improve your understanding of the algorithm's flow. Besides, below is step by step algorithm's description.
+
+Let's imagine, you want to go to the shop. Your house is A vertex and there are 4 possible stores around your house. How to find the closest one/ones? Luckily, you have graph, that connects your house with all these stores. So, you know what to do :)


Let's focus on 1 example, you mentioned that you wanted to find the shortest path from your house to your job.

kelvinlauKL · 2017-05-01T23:59:30Z

Dijkstra Algorithm/README.md

+
+## Initialization
+
+When the algorithm starts to work initial graph looks like this:


This sentence doesn't make sense.

In the intro, your example was about going from your house to your [job] company office. It'll read much better to refocus the reader on that example.

This diagram is a bit too noisy to start off with. Let's simplify it a bit. For instance, you could start without the weights of the graph:

Explain what the lines are, and what the circles represents.

kelvinlauKL · 2017-05-02T00:20:38Z

Dijkstra Algorithm/README.md

+
+<img src="Images/image1.png" height="250" />
+
+The table below represents graph state:


Instead of jumping straight to a table, gradually warm up the reader by introducing the concept of a weighted graph. This is where you update your diagram with more information:

For simplicity, tell the reader this is only a one way street (directed graph).

kelvinlauKL · 2017-05-02T00:25:28Z

Dijkstra Algorithm/README.md

+| Visited | F | F | F | F | F |
+| Path Length From Start | inf | inf | inf | inf | inf |
+| Path Vertices From Start | [ ] | [ ] | [ ] | [ ] | [ ] |
+


Before you introduce this table for computing Dijkstra's algorithm, its sometimes helpful to give the reader a concrete goal to think about. Tell them their goal is to find the shortest path from A to E:

Afterwards, you can talk about how to reason about it. That's where the table comes into play.

kelvinlauKL · 2017-05-02T00:33:50Z

Dijkstra Algorithm/README.md

+Then we check all of its neighbors. 
+If checking vertex path length from start + edge weigth is smaller than neighbor's path length from start, then we set neighbor's path length from start new value and append to its pathVerticesFromStart array new vertex: checkingVertex. Repeat this action for every vertex.
+
+for clarity:


Diagrams here will help a lot. You could do something like this:

This diagram focuses the reader on what you're currently trying to show (the path from A to B), while phasing out everything else. You'll leave the goal (Vertex E) in plain sight to remind the reader of the goal.

kelvinlauKL · 2017-05-02T00:38:47Z

Dijkstra Algorithm/README.md

+
+<img src="Images/image3.png" height="250" />
+
+And its state is here:


For the rest of the steps, you should use these graphs to showcase the rest of the maneuvers, taking it 1 step at a time:

wow-such-amazing · 2017-05-02T07:58:34Z

Thanks a lot again) But I'm not agree with concept, that it would be better to use directed graph. Because, about 2 months ago I was looking for algorithm, that will be possible to use in not directed graph. And I thought, that Dijkstra won't satisfy my requirements, because explanations mostly used directed graphs, but I needed to go in both directions on every edge.
Maybe it would be better to add both directed and not directed graph explanations?

kelvinlauKL · 2017-05-02T09:58:48Z

Interesting. Dijkstra's algorithm should be functionally identical for both undirected and directed graphs. I can't picture why you would want to go in both directions for every edge. You'd never want to travel back and forth since that only increases your total distance.

Could you explain why you needed to go in both directions using Dijkstra's?

wow-such-amazing · 2017-05-02T10:33:29Z

In my app I have a graph, that is not directed at all. You can go in both directions on every edge. And when I was searching for suitable algorithm, I've seen some explanations with directed graph and I was like: "That won't work in my case, because I have not directed weighted graph". But it would, because it works for not directed graphs to.

wow-such-amazing · 2017-05-02T10:34:34Z

In a park, for example, people can go in both directions on footpaths.

kelvinlauKL · 2017-05-03T01:03:31Z

Let's take this step by step. If you want to find the shortest path between 2 vertexes in a graph, would you ever want to travel down a single undirected edge more than once?

wow-such-amazing · 2017-05-03T06:35:07Z

Probably I won't. But I don't know whether i'll travel up or down an edge. So it doesn't make sense to create directed graph with both directions on every edge.

kelvinlauKL · 2017-05-03T06:56:44Z

Can you elaborate on that? One of the most common ways to represent an undirected graph is to use a directed graph where every edge is paired with another edge going in the opposite direction.

wow-such-amazing · 2017-05-03T08:00:54Z

But why? It is extra work, isn't it? :)

wow-such-amazing · 2017-05-03T08:01:41Z

If I have not directed graph, why should I make it directed?

kelvinlauKL · 2017-05-03T08:50:00Z

@crabman448 I took a look at your implementation, here's what I mean:

func setupConnections() { // ... let neighbor1 = (neighborVertex, randomWeight) let neighbor2 = (vertex, randomWeight) vertex.neighbors.append(neighbor1) neighborVertex.neighbors.append(neighbor2) }

neighbor1 and neighbor2 are your directed edges. They represent the destination and the weight of the edge. Your undirected graph is built by using a pair of directed edges. Every connection you make is done by adding a pair of edges.

You're right about it being extra work - hence I suggest we base the explanation based on a directed graph.

To create a directed graph, all you need to do is remove one of the pairs:

func setupConnections() { // ... let neighbor1 = (neighborVertex, randomWeight) vertex.neighbors.append(neighbor1) }

Now your graph is a directed graph, with exactly half as many edges than your previous undirected graph. I recommend reading our tutorial on adjacency lists for more info.

wow-such-amazing · 2017-05-03T09:10:29Z

Hmm, got you)
"In an undirected graph, you can treat it as a directed graph that goes both ways."
So, when all graph's edges are bidirectional, then it is still directed graph? And there is no other way to achieve such behaviour?

kelvinlauKL · 2017-05-03T09:21:38Z

Yes

An undirected graph is a directed graph where every edge has another edge going in the opposite direction

All undirected graphs can be viewed as a directed graph.
A directed graph is undirected if and only if every edge is paired with an edge going in the opposite direction

wow-such-amazing · 2017-05-03T09:25:24Z

Things have become clearer)) Thank you again. I will mention this statement in the beginning of the readme.

wow-such-amazing · 2017-05-10T10:01:43Z

I'm currently busy with my graduating project in the university. I'll fix description as soon as I will have enough time)

scha00 · 2017-05-22T01:55:45Z

Awesome work. I can see both of you are diligently making progress over many days. Looking forward to see this completed.

wow-such-amazing · 2017-05-24T13:09:55Z

I'm back now :)
Hope, that I will finish in a couple of days.

Edits

wow-such-amazing · 2017-05-24T15:11:50Z

I've added some general concepts about graphs, but I would like to use example with shops and choosing the closest one of them, because it correlates better with Dijkstra's algorithm: Shortest paths from source vertex to all others.
I'll check grammar, punctuation and spelling mistakes soon)

wow-such-amazing · 2017-06-12T02:31:02Z

@kelvinlauKL I've checked grammar etc and hope it is ready for merge now :)

kelvinlauKL · 2017-06-12T08:29:50Z

Nice. I just did a quick read and it's shaping up very nicely. Great improvements!

I'll do a more thorough review in the next few days.

kelvinlauKL · 2017-06-20T04:46:26Z

Woot, I had a chance to read over it. It's much nicer than before. This is ready for a merge.

I think there's still lots of room for improvement in areas like readability, so if you have time, consider spending more time improving the readme files. Thanks @crabman448!

Taras Nikulin added 2 commits April 4, 2017 10:08

Dijkstra's algorithm

b0204d3

Deleted unnecessary code and added comments

777bf7b

kelvinlauKL self-assigned this Apr 13, 2017

kelvinlauKL requested changes Apr 13, 2017

View reviewed changes

Taras Nikulin added 20 commits April 17, 2017 12:11

Merge remote-tracking branch 'raywenderlich/master' into dijkstra

9062c53

Removed extra screenshots

42db9a9

Credits to the bottom

a6020a5

Moved Vertex and Dijkstra classes to sources

a76764e

Hashable, Equatable to Extensions

3bdb63d

Removed simplicity unwrapped for initialized variable

b2c5674

Removed unnecessary explicit typing

f0d057b

Refactor Equatable extension

d0c6888

Made vertex optional

222e96d

Removed unnecessary self

e256103

Removed self, unnecessary imports

5060d78

Replaced with optional

900b7ca

Refactored a bit

2a1a2a1

Updated description

8461017

Added gif

f7793cc

Added link

3f6be33

Updated link

a153067

Revert "Updated link"

b59ffa5

This reverts commit a153067.

Revert "Added link"

ef13b50

This reverts commit 3f6be33.

Added link to Wikipedia

aea01e8

This reverts commit a153067.

kelvinlauKL requested changes Apr 17, 2017

View reviewed changes

Replaced _vertices with vertices

149515f

kelvinlauKL requested changes May 2, 2017

View reviewed changes

Taras Nikulin added 2 commits May 24, 2017 16:01

Merge remote-tracking branch 'raywenderlich/master' into dijkstra

3e301e8

Code improvements

0a813ba

Taras Nikulin and others added 5 commits May 24, 2017 17:54

Updated description

35db21b

Fix

e901736

fix

b1a50b4

Fixes

bb8b2a7

Merge pull request #2 from crabman448/edits

3f25101

Edits

Language fixes

5428e53

kelvinlauKL merged commit 5f4e339 into kodecocodes:master Jun 20, 2017


		## Screenshots

		<img src="Screenshots/screenshot1.jpg" height="400" />

		@@ -0,0 +1,137 @@
		//: Playground - noun: a place where people can play

		import Foundation


		open class Vertex: Hashable, Equatable {

		open var identifier: String!

		@@ -0,0 +1,45 @@
		# Dijkstra-algorithm

		## About this repository


		<img src="Images/Dijkstra_Animation.gif" height="250" />

		[Wikipedia](https://en.wikipedia.org/wiki/Dijkstra%27s_algorithm)'s explanation:


		## Dijkstra's algorithm explanation

		<img src="Images/Dijkstra_Animation.gif" height="250" />


		Let's imagine, you want to go to the shop. Your house is A vertex and there are 4 possible stores around your house. How to find the closest one/ones? Luckily, you have graph, that connects your house with all these stores. So, you know what to do :)

		## Initialization


		I have created VisualizedDijkstra.playground to improve your understanding of the algorithm's flow. Besides, below is step by step algorithm's description.

		Let's imagine, you want to go to the shop. Your house is A vertex and there are 4 possible stores around your house. How to find the closest one/ones? Luckily, you have graph, that connects your house with all these stores. So, you know what to do :)


		## Initialization

		When the algorithm starts to work initial graph looks like this:


		<img src="Images/image1.png" height="250" />

		The table below represents graph state:


		<img src="Images/image3.png" height="250" />

		And its state is here:

Dijkstra's algorithm #428

Dijkstra's algorithm #428

Uh oh!

Conversation

wow-such-amazing commented Apr 4, 2017

wow-such-amazing commented Apr 12, 2017

kelvinlauKL commented Apr 13, 2017

kelvinlauKL left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wow-such-amazing commented Apr 16, 2017

kelvinlauKL left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wow-such-amazing commented Apr 17, 2017

kelvinlauKL left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wow-such-amazing commented May 2, 2017

kelvinlauKL commented May 2, 2017

wow-such-amazing commented May 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

wow-such-amazing commented May 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

kelvinlauKL commented May 3, 2017

wow-such-amazing commented May 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

kelvinlauKL commented May 3, 2017

wow-such-amazing commented May 3, 2017

wow-such-amazing commented May 3, 2017

kelvinlauKL commented May 3, 2017

wow-such-amazing commented May 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

kelvinlauKL commented May 3, 2017

wow-such-amazing commented May 3, 2017

wow-such-amazing commented May 10, 2017

scha00 commented May 22, 2017

wow-such-amazing commented May 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

wow-such-amazing commented May 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

wow-such-amazing commented Jun 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

kelvinlauKL commented Jun 12, 2017

kelvinlauKL commented Jun 20, 2017

Labels

3 participants

kelvinlauKL left a comment •

edited

Loading

wow-such-amazing commented May 2, 2017 •

edited

Loading

wow-such-amazing commented May 2, 2017 •

edited

Loading

wow-such-amazing commented May 3, 2017 •

edited

Loading

wow-such-amazing commented May 3, 2017 •

edited

Loading

wow-such-amazing commented May 24, 2017 •

edited

Loading

wow-such-amazing commented May 24, 2017 •

edited

Loading

wow-such-amazing commented Jun 12, 2017 •

edited

Loading