change service#Start to return just error (Refs #45)

```
@melekes
yeah, bool is superfluous
@ethanfrey
If I remember correctly when I was writing test code, if I call Start() on a Service that is already running, it returns (false, nil). Only if I try to legitimately start it, but it fails in startup do I get an error.
The distinction is quite important to make it safe for reentrant calls. The other approach would be to have a special error type like ErrAlreadyStarted, then check for that in your code explicitly. Kind of like if I make a db call in gorm, and get an error, I check if it is a RecordNotFound error, or whether there was a real error with the db query.
@melekes
Ah, I see. Thanks. I must say I like ErrAlreadyStarted approach more (not just in Golang)
```
This commit is contained in:
Anton Kaliaev
2017-11-06 12:18:04 -05:00
parent ac0cf0b2e2
commit 4123d54bf6
2 changed files with 26 additions and 20 deletions
+12 -6
View File
@@ -1,13 +1,19 @@
package common
import (
"errors"
"sync/atomic"
"github.com/tendermint/tmlibs/log"
)
var (
ErrAlreadyStarted = errors.New("already started")
ErrAlreadyStopped = errors.New("already stopped")
)
type Service interface {
Start() (bool, error)
Start() error
OnStart() error
Stop() bool
@@ -94,11 +100,11 @@ func (bs *BaseService) SetLogger(l log.Logger) {
}
// Implements Servce
func (bs *BaseService) Start() (bool, error) {
func (bs *BaseService) Start() error {
if atomic.CompareAndSwapUint32(&bs.started, 0, 1) {
if atomic.LoadUint32(&bs.stopped) == 1 {
bs.Logger.Error(Fmt("Not starting %v -- already stopped", bs.name), "impl", bs.impl)
return false, nil
return ErrAlreadyStopped
} else {
bs.Logger.Info(Fmt("Starting %v", bs.name), "impl", bs.impl)
}
@@ -106,12 +112,12 @@ func (bs *BaseService) Start() (bool, error) {
if err != nil {
// revert flag
atomic.StoreUint32(&bs.started, 0)
return false, err
return err
}
return true, err
return nil
} else {
bs.Logger.Debug(Fmt("Not starting %v -- already started", bs.name), "impl", bs.impl)
return false, nil
return ErrAlreadyStarted
}
}