From 64fbb923cae22bb6fd6e31843fe940f4674e227f Mon Sep 17 00:00:00 2001
From: Lioncash <mathew1800@gmail.com>
Date: Sat, 24 Aug 2019 22:39:10 -0400
Subject: [PATCH 1/4] MIDIDecoder: Make readContinuedValue an internal function

This doesn't rely on any member state, so it can be decoupled from the
interface entirely.
---
 include/boo/audiodev/MIDIDecoder.hpp |  2 --
 lib/audiodev/MIDIDecoder.cpp         | 11 ++++++-----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/boo/audiodev/MIDIDecoder.hpp b/include/boo/audiodev/MIDIDecoder.hpp
index 79a1047..8ce82bb 100644
--- a/include/boo/audiodev/MIDIDecoder.hpp
+++ b/include/boo/audiodev/MIDIDecoder.hpp
@@ -9,8 +9,6 @@ class IMIDIReader;
 class MIDIDecoder {
   IMIDIReader& m_out;
   uint8_t m_status = 0;
-  bool _readContinuedValue(std::vector<uint8_t>::const_iterator& it, std::vector<uint8_t>::const_iterator end,
-                           uint32_t& valOut);
 
 public:
   MIDIDecoder(IMIDIReader& out) : m_out(out) {}
diff --git a/lib/audiodev/MIDIDecoder.cpp b/lib/audiodev/MIDIDecoder.cpp
index e469fb8..d795272 100644
--- a/lib/audiodev/MIDIDecoder.cpp
+++ b/lib/audiodev/MIDIDecoder.cpp
@@ -7,11 +7,11 @@
 
 
 namespace boo {
-
+namespace {
 constexpr uint8_t clamp7(uint8_t val) { return std::max(0, std::min(127, int(val))); }
 
-bool MIDIDecoder::_readContinuedValue(std::vector<uint8_t>::const_iterator& it,
-                                      std::vector<uint8_t>::const_iterator end, uint32_t& valOut) {
+bool readContinuedValue(std::vector<uint8_t>::const_iterator& it, std::vector<uint8_t>::const_iterator end,
+                        uint32_t& valOut) {
   uint8_t a = *it++;
   valOut = a & 0x7f;
 
@@ -33,6 +33,7 @@ bool MIDIDecoder::_readContinuedValue(std::vector<uint8_t>::const_iterator& it,
 
   return true;
 }
+} // Anonymous namespace
 
 std::vector<uint8_t>::const_iterator MIDIDecoder::receiveBytes(std::vector<uint8_t>::const_iterator begin,
                                                                std::vector<uint8_t>::const_iterator end) {
@@ -52,7 +53,7 @@ std::vector<uint8_t>::const_iterator MIDIDecoder::receiveBytes(std::vector<uint8
       a = *it++;
 
       uint32_t length;
-      _readContinuedValue(it, end, length);
+      readContinuedValue(it, end, length);
       it += length;
     } else {
       uint8_t chan = m_status & 0xf;
@@ -125,7 +126,7 @@ std::vector<uint8_t>::const_iterator MIDIDecoder::receiveBytes(std::vector<uint8
         switch (Status(m_status & 0xff)) {
         case Status::SysEx: {
           uint32_t len;
-          if (!_readContinuedValue(it, end, len) || end - it < len)
+          if (!readContinuedValue(it, end, len) || end - it < len)
             return begin;
           m_out.sysex(&*it, len);
           break;

From e48a09419801f46d60637b50618a7bc943ffe15e Mon Sep 17 00:00:00 2001
From: Lioncash <mathew1800@gmail.com>
Date: Sat, 24 Aug 2019 22:44:31 -0400
Subject: [PATCH 2/4] MIDIDecoder: Convert return value of readContinuedValue
 into a std::optional

Rather than use an out reference, we can convert the return value into a
std::optional, combining the out reference and boolean return value into
one.
---
 lib/audiodev/MIDIDecoder.cpp | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/lib/audiodev/MIDIDecoder.cpp b/lib/audiodev/MIDIDecoder.cpp
index d795272..551323b 100644
--- a/lib/audiodev/MIDIDecoder.cpp
+++ b/lib/audiodev/MIDIDecoder.cpp
@@ -1,6 +1,7 @@
 #include "boo/audiodev/MIDIDecoder.hpp"
 
 #include <algorithm>
+#include <optional>
 
 #include "boo/audiodev/IMIDIReader.hpp"
 #include "lib/audiodev/MIDICommon.hpp"
@@ -10,28 +11,30 @@ namespace boo {
 namespace {
 constexpr uint8_t clamp7(uint8_t val) { return std::max(0, std::min(127, int(val))); }
 
-bool readContinuedValue(std::vector<uint8_t>::const_iterator& it, std::vector<uint8_t>::const_iterator end,
-                        uint32_t& valOut) {
+std::optional<uint32_t> readContinuedValue(std::vector<uint8_t>::const_iterator& it,
+                                           std::vector<uint8_t>::const_iterator end) {
   uint8_t a = *it++;
-  valOut = a & 0x7f;
+  uint32_t valOut = a & 0x7f;
 
-  if (a & 0x80) {
-    if (it == end)
-      return false;
+  if ((a & 0x80) != 0) {
+    if (it == end) {
+      return std::nullopt;
+    }
     valOut <<= 7;
     a = *it++;
     valOut |= a & 0x7f;
 
-    if (a & 0x80) {
-      if (it == end)
-        return false;
+    if ((a & 0x80) != 0) {
+      if (it == end) {
+        return std::nullopt;
+      }
       valOut <<= 7;
       a = *it++;
       valOut |= a & 0x7f;
     }
   }
 
-  return true;
+  return valOut;
 }
 } // Anonymous namespace
 
@@ -52,9 +55,8 @@ std::vector<uint8_t>::const_iterator MIDIDecoder::receiveBytes(std::vector<uint8
         return begin;
       a = *it++;
 
-      uint32_t length;
-      readContinuedValue(it, end, length);
-      it += length;
+      const auto length = readContinuedValue(it, end);
+      it += *length;
     } else {
       uint8_t chan = m_status & 0xf;
       switch (Status(m_status & 0xf0)) {
@@ -125,10 +127,11 @@ std::vector<uint8_t>::const_iterator MIDIDecoder::receiveBytes(std::vector<uint8
       case Status::SysEx: {
         switch (Status(m_status & 0xff)) {
         case Status::SysEx: {
-          uint32_t len;
-          if (!readContinuedValue(it, end, len) || end - it < len)
+          const auto len = readContinuedValue(it, end);
+          if (!len || end - it < *len) {
             return begin;
-          m_out.sysex(&*it, len);
+          }
+          m_out.sysex(&*it, *len);
           break;
         }
         case Status::TimecodeQuarterFrame: {

From e7fe7ff49fea70e97becf23f247dfe776b91e3ca Mon Sep 17 00:00:00 2001
From: Lioncash <mathew1800@gmail.com>
Date: Sat, 24 Aug 2019 22:47:06 -0400
Subject: [PATCH 3/4] MIDIDecoder: Make use of std::clamp() within clamp7()

We can simplify the implementation of clamp7() with the use of
std::clamp().
---
 lib/audiodev/MIDIDecoder.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/audiodev/MIDIDecoder.cpp b/lib/audiodev/MIDIDecoder.cpp
index 551323b..40eca9c 100644
--- a/lib/audiodev/MIDIDecoder.cpp
+++ b/lib/audiodev/MIDIDecoder.cpp
@@ -9,7 +9,7 @@
 
 namespace boo {
 namespace {
-constexpr uint8_t clamp7(uint8_t val) { return std::max(0, std::min(127, int(val))); }
+constexpr uint8_t clamp7(uint8_t val) { return std::clamp(val, uint8_t{0}, uint8_t{127}); }
 
 std::optional<uint32_t> readContinuedValue(std::vector<uint8_t>::const_iterator& it,
                                            std::vector<uint8_t>::const_iterator end) {

From 068ff29b44a6221f67481e1654814ee3efa7c2af Mon Sep 17 00:00:00 2001
From: Lioncash <mathew1800@gmail.com>
Date: Sat, 24 Aug 2019 22:49:13 -0400
Subject: [PATCH 4/4] MIDIDecoder: Use auto for iterator type

Same thing, significantly less reading.
---
 lib/audiodev/MIDIDecoder.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/audiodev/MIDIDecoder.cpp b/lib/audiodev/MIDIDecoder.cpp
index 40eca9c..4f1ded3 100644
--- a/lib/audiodev/MIDIDecoder.cpp
+++ b/lib/audiodev/MIDIDecoder.cpp
@@ -40,7 +40,7 @@ std::optional<uint32_t> readContinuedValue(std::vector<uint8_t>::const_iterator&
 
 std::vector<uint8_t>::const_iterator MIDIDecoder::receiveBytes(std::vector<uint8_t>::const_iterator begin,
                                                                std::vector<uint8_t>::const_iterator end) {
-  std::vector<uint8_t>::const_iterator it = begin;
+  auto it = begin;
   while (it != end) {
     uint8_t a = *it++;
     uint8_t b;