ants
ants copied to clipboard
allow NewPoolWithFunc can invoke with nil argument
- abstract a Task interface for SubmitTask which is more convenient
name: Pull request about: Propose changes to the code title: 'allow NewPoolWithFunc can invoke with nil argument' labels: '' assignees: ''
1. Are you opening this pull request for bug-fixs, optimizations or new feature?
optimizations
2. Please describe how these code changes achieve your intention.
- allow NewPoolWithFunc can invoke with nil argument
- abstract a Task interface for SubmitTask which is more convenient
3. Please link to the relevant issues (if any).
none
4. Which documentation changes (if any) need to be made/updated because of this PR?
none
4. Checklist
- [x] I have squashed all insignificant commits.
- [ ] I have commented my code for explaining package types, values, functions, and non-obvious lines.
- [x] I have written unit tests and verified that all tests passes (if needed).
- [ ] I have documented feature info on the README (only when this PR is adding a new feature).
- [ ] (optional) I am willing to help maintain this change if there are issues with it later.
This PR could introduce a problem.
// The address of two struct{} values may be the same.
var a, b struct{}
fmt.Println(&a == &b) // true
My case
package main
import (
"fmt"
)
var stopArg interface{} = &struct{}{}
var stopArg1 interface{} = &struct{}{}
var stopArg2 interface{} = &struct{}{}
func main() {
check(stopArg1)
check(stopArg2)
}
func check(iface interface{}) {
if iface == stopArg {
fmt.Println("RETURN")
return
}
fmt.Println("RUN")
}
OUTPUT
RETURN
RETURN
[Process exited 0]
I'm sorry that I offended you. That wasn't my intent.
@bingoohuang
@Mutated1994 thanks for your pointing out the potential bug for the stopArg definition
I fixed to var stopArg interface{} = &struct{ a byte }{}
and the test cases like
func TestAntsPoolWithFuncNilParam(t *testing.T) {
var old1 interface{} = &struct{ a byte }{}
var old2 interface{} = &struct{ a byte }{}
assert.True(t, old1 != old2)
isStopArg := func(v interface{}) bool { return v == stopArg }
var a1 interface{} = &struct{ a byte }{}
var a2 interface{} = &struct{ a byte }{}
var a3 interface{} = &struct{ a byte }{}
assert.False(t, a1 == a2)
assert.False(t, isStopArg(a1))
assert.False(t, isStopArg(a2))
assert.False(t, isStopArg(a3))
assert.True(t, isStopArg(stopArg))
}
please have a code view for the last change, thanks very much.
@Mutated1994 thanks for your pointing out the potential bug for the stopArg definition
I fixed to
var stopArg interface{} = &struct{ a byte }{}
and the test cases likefunc TestAntsPoolWithFuncNilParam(t *testing.T) { var old1 interface{} = &struct{ a byte }{} var old2 interface{} = &struct{ a byte }{} assert.True(t, old1 != old2) isStopArg := func(v interface{}) bool { return v == stopArg } var a1 interface{} = &struct{ a byte }{} var a2 interface{} = &struct{ a byte }{} var a3 interface{} = &struct{ a byte }{} assert.False(t, a1 == a2) assert.False(t, isStopArg(a1)) assert.False(t, isStopArg(a2)) assert.False(t, isStopArg(a3)) assert.True(t, isStopArg(stopArg)) }
please have a code view for the last change, thanks very much.
Dude, it's my pleasure.
LGTM
Why does ants
need this?
manbe you can ask, why does not ants support this.
There is no need for ants to limit users to pass a nil argument to their register function as some special meaning.
Just because the fact that we can do, doesn't mean we should do.
I'm afraid you'll have to prove that this PR is necessary for users, BTW, I also believe there are ways to achieve the same effect based on the existing APIs.
Just because the fact that we can do, doesn't mean we should do.
I'm afraid you'll have to prove that this PR is necessary for users, BTW, I also believe there are ways to achieve the same effect based on the existing APIs.
TBH, I couldn't agree more.
Just because the fact that we can do, doesn't mean we should do.
I'm afraid you'll have to prove that this PR is necessary for users, BTW, I also believe there are ways to achieve the same effect based on the existing APIs.
Yeah, I agree with you, but it's the user's choice to make a decision to do or not to do, not the common library or the language itself.
TLDR, It's a user's duty, not the ants'.