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

refs #14: implement FindFirstElementGreaterOrEqualToKey #15

Merged
merged 2 commits into from
Sep 24, 2021

Conversation

someblue
Copy link

No description provided.

Change-Id: Iac0725a351c0efb24319a95e39d4fbf5e329d6a3
Copy link
Owner

@huandu huandu left a comment

Choose a reason for hiding this comment

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

感谢快速提供了一个 PR,我提了几个代码设计和风格的建议,辛苦修改下,欢迎在 issue 里面讨论。

skiplist.go Outdated
// If there is no such element, returns nil.
//
// The complexity is O(log(N)).
func (list *SkipList) FindFirstElementGreaterOrEqualToKey(key interface{}) (elem *Element) {
Copy link
Owner

Choose a reason for hiding this comment

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

这个接口的名字太长了,希望改成 Find,注释里写好这个函数的功能即可,不用那么长的名字。

Copy link
Owner

Choose a reason for hiding this comment

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

另外建议提供两个公开方法:

// 相当于调用 list.FindNext(nil, key)
func (list *SkipList) Find(key interface{}) (elem *Element)

// 从 elem 开始查找下一个符合要求的 key,elem 自身不包含。
func (list *SkipList) FindNext(elem *Element, key interface{}) (elem *Element)

另外提供一个私有方法,将计算好的 score 放进去,减少 calcScore 调用。

func (list *SkipList) findNext(start *Element, score float64, key interface{}) (elem *Element)

skiplist.go Outdated
if list.length == 0 {
firstElem := list.FindFirstElementGreaterOrEqualToKey(key)
if firstElem == nil {
elem = nil
Copy link
Owner

Choose a reason for hiding this comment

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

没必要额外给 elem = nil,默认值就是 nil。

skiplist.go Outdated

if score < list.Front().score || score > list.Back().score {
if list.compare(score, key, firstElem) != 0 {
elem = nil
Copy link
Owner

Choose a reason for hiding this comment

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

同上,不需要赋值。

skiplist.go Outdated
@@ -243,41 +243,19 @@ func (list *SkipList) Set(key, value interface{}) (elem *Element) {
//
// The complexity is O(log(N)).
func (list *SkipList) Get(key interface{}) (elem *Element) {
if list.length == 0 {
firstElem := list.FindFirstElementGreaterOrEqualToKey(key)
Copy link
Owner

Choose a reason for hiding this comment

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

由于 Find 函数里面调用 calcScore 计算了 score,下面的代码再次计算了一次,有些浪费,建议写一个内部函数 findNext(start *Element, score float64, key interface{}) (elem *Element),传入计算好的 score,省去额外的 calcScore。

Change-Id: Id2b9a018b2700133fbd5186f60a03c45fd18d80d
@someblue
Copy link
Author

@huandu hello 感谢 comments,已调整代码

@huandu huandu merged commit 1b870b1 into huandu:master Sep 24, 2021
@huandu
Copy link
Owner

huandu commented Sep 24, 2021

没有问题了,已经合并。

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