From 0bd023a130d73a3db0d8ad903e1212e790b70d98 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Frank=20Schr=C3=B6der?= <frank.schroeder@northvolt.com>
Date: Mon, 22 May 2023 14:18:27 +0200
Subject: [PATCH 1/2] Remove 'if err == nil' anti-pattern

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.
---
 client.go       | 23 ++++++++++++-----------
 stats/stats.go  |  5 ++---
 subscription.go | 17 +++++++++--------
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/client.go b/client.go
index 29fac722..42bbf657 100644
--- a/client.go
+++ b/client.go
@@ -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
 }
diff --git a/stats/stats.go b/stats/stats.go
index 25c97786..6e7125cf 100644
--- a/stats/stats.go
+++ b/stats/stats.go
@@ -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):
diff --git a/subscription.go b/subscription.go
index 5cde5835..15f9a397 100644
--- a/subscription.go
+++ b/subscription.go
@@ -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

From eb0d48d0a388d6dad7e109a36cc0e21a3d3fe372 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Frank=20Schr=C3=B6der?= <frank.schroeder@northvolt.com>
Date: Mon, 22 May 2023 14:48:42 +0200
Subject: [PATCH 2/2] review comments

---
 client.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/client.go b/client.go
index 42bbf657..2c11096c 100644
--- a/client.go
+++ b/client.go
@@ -1029,7 +1029,6 @@ func (c *Client) ReadWithContext(ctx context.Context, req *ua.ReadRequest) (*ua.
 	var res *ua.ReadResponse
 	err := c.SendWithContext(ctx, req, func(v interface{}) error {
 		err := safeAssign(v, &res)
-
 		if err != nil {
 			return err
 		}