Skip to content

Commit

Permalink
Remove 'if err == nil' anti-pattern
Browse files Browse the repository at this point in the history
Using 'if err == nil' is an anti-pattern in Go and should be avoided
since most if not all Go developers expect 'if err != nil {...}'

This patch flips/removes the checks in the main client code but leaves
the checks in the test cases were we require an error.
  • Loading branch information
magiconair committed May 22, 2023
1 parent 6fd3deb commit 0bd023a
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 22 deletions.
23 changes: 12 additions & 11 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1030,24 +1030,25 @@ func (c *Client) ReadWithContext(ctx context.Context, req *ua.ReadRequest) (*ua.
err := c.SendWithContext(ctx, req, func(v interface{}) error {
err := safeAssign(v, &res)

if err != nil {
return err
}

// If the client cannot decode an extension object then its
// value will be nil. However, since the EO was known to the
// server the StatusCode for that data value will be OK. We
// therefore check for extension objects with nil values and set
// the status code to StatusBadDataTypeIDUnknown.
if err == nil {
for _, dv := range res.Results {
if dv.Value == nil {
continue
}
val := dv.Value.Value()
if eo, ok := val.(*ua.ExtensionObject); ok && eo.Value == nil {
dv.Status = ua.StatusBadDataTypeIDUnknown
}
for _, dv := range res.Results {
if dv.Value == nil {
continue
}
val := dv.Value.Value()
if eo, ok := val.(*ua.ExtensionObject); ok && eo.Value == nil {
dv.Status = ua.StatusBadDataTypeIDUnknown
}
}

return err
return nil
})
return res, err
}
Expand Down
5 changes: 2 additions & 3 deletions stats/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,10 @@ func (s *Stats) Reset() {

// RecordError updates the metric for an error by one.
func (s *Stats) RecordError(err error) {
if err == nil {
return
}
var code ua.StatusCode
switch {
case err == nil:
return
case errors.Is(err, io.EOF):
s.Error.Add("io.EOF", 1)
case errors.Is(err, ua.StatusOK):
Expand Down
17 changes: 9 additions & 8 deletions subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,17 +173,18 @@ func (s *Subscription) UnmonitorWithContext(ctx context.Context, monitoredItemID
err := s.c.SendWithContext(ctx, req, func(v interface{}) error {
return safeAssign(v, &res)
})
if err != nil {
return nil, err
}

if err == nil {
// remove monitored items
s.itemsMu.Lock()
for _, id := range monitoredItemIDs {
delete(s.items, id)
}
s.itemsMu.Unlock()
// remove monitored items
s.itemsMu.Lock()
for _, id := range monitoredItemIDs {
delete(s.items, id)
}
s.itemsMu.Unlock()

return res, err
return res, nil
}

// Note: Starting with v0.5 this method will require a context
Expand Down

0 comments on commit 0bd023a

Please sign in to comment.