Skip to content

Conversation

@XQ-Gang
Copy link
Collaborator

@XQ-Gang XQ-Gang commented Jul 6, 2025

  1. add ContainsBy
  2. add OrO
  3. goption/gresult不依赖别的包,防止循环依赖

@XQ-Gang XQ-Gang requested review from ltoddy, unnamed42 and xiekeyi98 July 6, 2025 17:26
@codecov
Copy link

codecov bot commented Jul 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.13%. Comparing base (f5f07b9) to head (968a140).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #18      +/-   ##
==========================================
+ Coverage   95.79%   96.13%   +0.34%     
==========================================
  Files          34       34              
  Lines        4443     4451       +8     
==========================================
+ Hits         4256     4279      +23     
+ Misses        144      131      -13     
+ Partials       43       41       -2     
Flag Coverage Δ
go-1.18.x 94.79% <100.00%> (-0.13%) ⬇️
go-1.19.x 94.93% <100.00%> (+<0.01%) ⬆️
go-1.20.x 95.25% <100.00%> (-0.06%) ⬇️
go-1.21.x 95.52% <100.00%> (-0.13%) ⬇️
go-1.22.x 95.52% <100.00%> (+0.21%) ⬆️
go-1.23.x 95.39% <100.00%> (+0.14%) ⬆️
go-1.24.x 95.32% <100.00%> (+0.14%) ⬆️
unittests 96.13% <100.00%> (+0.34%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@XQ-Gang XQ-Gang requested a review from SilverRainZ July 11, 2025 06:13
Copy link
Member

Choose a reason for hiding this comment

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

OrO 这个函数名称能直观的表达出你想要的函数意思吗?

Copy link
Member

Choose a reason for hiding this comment

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

OrO 最后一个 O,是啥单词的缩写?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Option。类似 gsync中的LoadO、gconv中的ToR

func Err[T any](e error) R[T] {
if gvalue.IsNil(e) {
if reflect.ValueOf(e).IsNil() {
panic(fmt.Errorf("expected a non-nil error, but found nil error with type %T", e))
Copy link
Collaborator

Choose a reason for hiding this comment

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

想说这样性能会差不少,转头一看 gvalue.IsNil 也已经是反射了(

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

有个问题,原来的gvalue.IsNil()识别不了nil slice

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

有啥好办法吗

Copy link
Collaborator

Choose a reason for hiding this comment

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

场景是啥?我印象 var t []Tvar t = []T{} 在使用上应该完全没区别才对?(或者说不会导致什么问题)

Copy link
Collaborator Author

@XQ-Gang XQ-Gang Jul 11, 2025

Choose a reason for hiding this comment

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

从使用上,我也觉得不应该使用 IsNil 或者直接 == nil 来区分空slice。
但是从语义上,IsNil应该能区分出slice是否是nil,之前完全区分不出来,误导性比较强。
之前的实现:

var t []T
gvalue.IsNil(t) // false

Copy link
Collaborator

Choose a reason for hiding this comment

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

空 slice 就不是 nil 吧?

		var s []int
		assert.True(t, IsNil(s))
		var s2 = []int{}
		assert.False(t, IsNil(s2))

可以补一个测试。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes,现在的表现是你发的这样的。
image

r.err = errors.New(*jr.Err)
} else {
r.val = *jr.Val
r.err = nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里不能直接删除吧?把一个 var r Result 传给 json.Unarmal(data, &v) 后,是需要清除原来的值的?

这么一看发现原来也有问题 😂,应该在 L205 之前就把 r 清空了。

}
return goption.Nil[T]()
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

感觉这个函数名字不太直观,名字也不太清晰(还以为是 Or0)。能给一些更 real world 的用例么?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

主要是想迁移原先的choose.NotZero

Copy link
Collaborator

Choose a reason for hiding this comment

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

按原来规范可能叫 OfNonZero 比较好?不过还是不如 choose 直观。

Copy link
Collaborator

@SilverRainZ SilverRainZ left a comment

Choose a reason for hiding this comment

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

Round1

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.

4 participants