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

[WIP]尝试给list加Iter #234

Closed
wants to merge 1 commit into from

Conversation

fjmnwd
Copy link

@fjmnwd fjmnwd commented Dec 24, 2023

尝试给list加一个iterater,写了一个demo
用装饰器模式,在原有list接口上设计IterableList与Iter接口
一个IterableList最多拥有一个Iter,有Iter时候IterableList不许Add和Delete,其余写操作不禁止
Iter实现Delete方法时,采用懒删除的策略,Iter.Delete只是把对应的index放入deleteIndices数组中;在Iter.Next为空时,或用户主动Release这个Iter时,执行之前累积的的所有删除操作

@fjmnwd fjmnwd changed the title feat_list_iter 尝试给list加Iter Dec 24, 2023
Copy link

codecov bot commented Dec 25, 2023

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (45c4365) 95.91% compared to head (dab0ad1) 95.56%.

Files Patch % Lines
list/iterable_list.go 81.35% 7 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #234      +/-   ##
==========================================
- Coverage   95.91%   95.56%   -0.36%     
==========================================
  Files          60       61       +1     
  Lines        3233     3292      +59     
==========================================
+ Hits         3101     3146      +45     
- Misses        102      111       +9     
- Partials       30       35       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@longyue0521
Copy link
Collaborator

@fjmnwd

  1. 参考其他已合并请求代码修复下放“Check License”错误
  2. 在本地执行make setup 修复下方第二个错误“golangci-lint", 如果还不行,点击其后面的Details查看详情.

@fjmnwd
Copy link
Author

fjmnwd commented Dec 25, 2023

好的 这个不是最后打算合并的代码 只是一个草稿哈哈 之后修一下

Comment on lines +47 to +53
// IterableList List仅在没有Iter时可被编辑,一个Iter只能有一个Iter
type IterableList[T any] interface {
List[T]
GetIter() (Iter[T], bool)
releaseIter(func(list List[T]))
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. 注释需要重构,有些看不懂 —— “一个Iter只能有一个Iter”
  2. 都有了GetIter()方法, 那么我是不是通过得到的Iter[T].Release() 就可以释放了,为什么还要有releaseIter?

Copy link
Author

Choose a reason for hiding this comment

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

详细解释写在Iter的Issue里了
Iter有两个release方式,一个是取完了,Next不再有值;一个是用户主动释放
如果用户没有释放习惯,就靠releaseIter自动释放

break
}
}
iter, ok = testCase.list.GetIter()
Copy link
Collaborator

Choose a reason for hiding this comment

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

iter未被用到过

Copy link
Author

Choose a reason for hiding this comment

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

这个主要测试是iter被自然释放后(Next没有值),又可以重新GetIter

@longyue0521
Copy link
Collaborator

好的 这个不是最后打算合并的代码 只是一个草稿哈哈 之后修一下

如果是草稿的话, 建议修改标题为 “[WIP] 尝试给list加Iter" WIP表示work in progress, 当达到能后接受review得状态时去掉 [WIP]标志即可

@fjmnwd fjmnwd changed the title 尝试给list加Iter [WIP]尝试给list加Iter Dec 25, 2023
Copy link
Contributor

@flycash flycash left a comment

Choose a reason for hiding this comment

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

还是要从允许多个 Iterator 存在。那么问题就是:

  • 只允许一个 Iterator 修改。但是在允许多个 Iterator 存在的情况下,这个很难做到
  • 允许多个 Iterator 修改,交给用户去控制。用户应该知道多个修改的情况下,会出现问题。我们保证程序不会出现 panic(nil,或者下标越界之类的问题)就可以。

我倾向于第二种解决方案。也挺符合我说的,药医不死病,佛渡有缘人的说法

@@ -43,3 +43,16 @@ type List[T any] interface {
// AsSlice 每次调用都必须返回一个全新的切片
AsSlice() []T
}

// IterableList List仅在没有Iter时可被编辑,一个Iter只能有一个Iter
type IterableList[T any] interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

接口这样设计不太好。应该有一个独立的 Iterable 接口,而后 List 接口直接组合这个 Iterable 接口。

应该说,Iterable 是一个更加抽象的接口,也并不是独属于 List 的。

Copy link
Contributor

Choose a reason for hiding this comment

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

又或者直接在 List 上增加这个 Iterator 方法。

Copy link
Author

@fjmnwd fjmnwd Dec 27, 2023

Choose a reason for hiding this comment

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

接口这样设计不太好。应该有一个独立的 Iterable 接口,而后 List 接口直接组合这个 Iterable 接口。

应该说,Iterable 是一个更加抽象的接口,也并不是独属于 List 的。

这里想法是应用一个装饰器模式哈,就是对list的方法做一个封装,如果有Iter就不允许裸的List Add和Delete
IterableList的作用是把里面List的功能加一层保险(判断有没有Iter)

也可以把这个保险写在list里,但是这样所有实现都要改动,侵入性比较大

如果拆成两个接口的话,就只能采用侵入性的方案

Copy link
Author

Choose a reason for hiding this comment

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

我在issue里写得比较详细一些哈

@@ -43,3 +43,16 @@ type List[T any] interface {
// AsSlice 每次调用都必须返回一个全新的切片
AsSlice() []T
}

// IterableList List仅在没有Iter时可被编辑,一个Iter只能有一个Iter
Copy link
Contributor

Choose a reason for hiding this comment

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

一个 List 只能有一个这个其实目前来看不是很好。因为你可以预期大多数的使用场景否是读多写少,因此允许多个 Iterator 存在是很有必要的,但是修改的时候,可以考虑到只能有一个 Iterater 可以修改。

又或者我们直接把整个 Iterator 做成线程安全的,然后允许任意多个 Iterator 并发读写,交给用户来控制并发。也就是说,如果用户有多个 Gorotuine 通过 Iterator 来修改 List,那么它应该自己为自己负责。

Copy link
Author

@fjmnwd fjmnwd Dec 27, 2023

Choose a reason for hiding this comment

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

如果是读的话 为什么不直接走Get接口呢?
或者另外一个方案,拆两个Iter,一个叫Iter,另一个叫EditableIter,EditableIter只允许有一个?

Copy link
Contributor

Choose a reason for hiding this comment

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

是遍历,就是说有多个 iterator 同时遍历。可能中间是要判定元素是否符合某个条件,符合条件的就删除。

Copy link
Author

Choose a reason for hiding this comment

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

是遍历,就是说有多个 iterator 同时遍历。可能中间是要判定元素是否符合某个条件,符合条件的就删除。

还是觉得可以有多个Iter这个设定有点奇怪,List里大部分都不支持并发操作,有并发读一般就会有并发读与写吧。读多写少就是一个并发场景啊,和list设计感觉没啥关系
如果只是串行的,那只有一个Iter没什么问题啊,它遍历完了会自动释放的。释放完就可以重新创一个Iter。只是不允许两个Iter一起出现。而且Iter如果要实现Delete,那多个Iter一起出现就是并发读写了

type Iter[T any] interface {
Next() (T, bool)
Delete()
Release()
Copy link
Contributor

Choose a reason for hiding this comment

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

Release 是一个比较奇怪的方法,在只有一个 Iterator 的时候倒还好,但是我在前面说了,读多写少的环境不能做这个假设。

Copy link
Author

Choose a reason for hiding this comment

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

目前这个设计里面,Release这个接口用户可以不调用,不影响使用
Iter如果遍历完List就自动Release

"github.com/ecodeclub/ekit/internal/errs"
)

type IterableListImpl[T any] struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

基本上比较难提供统一的实现。就用 LinkedList 和 ArrayList 来说,Delete 的时候,对于 LinkedList 来说就是调整指针,但是对于 ArrayList 来说,就是调整切片的位置。

所以这边我个人会觉得需要分开来实现。

Copy link
Author

@fjmnwd fjmnwd Dec 27, 2023

Choose a reason for hiding this comment

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

因为删除采用的是异步删除(iter.Next为空 或者 主动Release时 删除),可以按index从后往前删,这样是可以实现的。只要允许传index来删除,这种方法对于多种实现都适应

Copy link
Contributor

Choose a reason for hiding this comment

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

不仅仅是适应的问题,还有性能的问题。正如同我说的,LinkedList 的删除操作,只需要调整指针,但是 Slice 的删除,就必须要挪动元素。

因此从这个意义上来说,其实没有很大的必要非得共享一个实现。完全可以不同的 list 有不同的 interator。

类似地道理,Iterator 也可以用于树形结构的遍历,那么删除也是需要自己实现的。

Copy link
Author

@fjmnwd fjmnwd Dec 27, 2023

Choose a reason for hiding this comment

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

我理解这个实现可以作为一个默认实现,为了性能还可以加树形/链表的实现

Copy link
Author

Choose a reason for hiding this comment

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

树形/链表的实现我理解也可以做到变成装饰器模式,而不对原始的list代码有改动,保持其整洁。只是一个提议哈

Copy link
Author

@fjmnwd fjmnwd Dec 27, 2023

Choose a reason for hiding this comment

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

我之后先按多Iter和分开实现试着改一下吧

Copy link
Contributor

Choose a reason for hiding this comment

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

可以先尝试针对 LinkedList 实现一个,这样会更加清晰。虽然我一直说装饰器很好用,但是这里我们的非功能特性要保证性能。

@flycash flycash closed this Apr 5, 2024
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