Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Aug 17, 2016

I am working on Skip List. It is almost finished.

@chris-pilcher
Copy link

Thanks @mitghi! 👍 Just let me know when it's ready for review.

Would you be able to use this style guide for writing code? E.g. change private func find_head(key:Key) to private func findHead(key: Key) and indent using 2 spaces.

@ghost
Copy link
Author

ghost commented Sep 1, 2016

Thanks @chris-pilcher . I add the description very soon.
And change the code style according to the style guide.

@kelvinlauKL
Copy link
Member

@mitghi Looking to cleanup the pull requests. What's the ETA on this being completed?

@ghost
Copy link
Author

ghost commented Sep 16, 2016

@kelvinlauKL I finish it this weekend.

@ghost
Copy link
Author

ghost commented Sep 17, 2016

@kelvinlauKL @chris-pilcher The code is finished and refactored. Please have a look, so I can work on it faster in case anything should get changed. Thanks

@ghost
Copy link
Author

ghost commented Oct 8, 2016

@kelvinlauKL @chris-pilcher :) I appreciate if you provide some feedback so I can improve it. Thanks.

@kelvinlauKL
Copy link
Member

@mitghi Sorry for the delay. I just did a review on your code. Great job overall.

  • well done on the usage of typealias on the generic parameters. Makes the code much prettier
  • nice work on generics
  • good amount of diagrams. They really help others understand the concepts

A couple of comments:

// not preferred
fileprivate var array = [T]()

// preferred
fileprivate var array: [T] = []

All extensions should be preceded with a // MARK: - Description of extension. This provides some context on why you're grouping up the stuff in an extension.

A couple of compiler errors on the latest Xcode 8.0. In particular, the coinFlip doesn't compile correctly.

// not preferred
interval func insert(key: Key, data: Payload) { ... }

// preferred
func insert(key: Key, data: Payload) { ... }

Everything in your file is prefixed with an invisible internal. There's no need to explicitly state it.

public func remove(key: Key) -> Void { ... }

No need to specify the return type as Void.

self.head
self.head = Node(asHead: true)

No need to specify self explicitly to reference properties, unless compiler requires it.

@ghost
Copy link
Author

ghost commented Oct 22, 2016

@kelvinlauKL Thank you very much Kelvin, I modify it accordingly.

EDIT:
I changed the code, it compiles both on linux and mac with swift 3.

@kelvinlauKL
Copy link
Member

@chris-pilcher Just letting you know I'm currently reviewing this.

@kelvinlauKL kelvinlauKL merged commit 313c4d3 into kodecocodes:master Dec 6, 2016
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.

3 participants