Small improvements (#27)

* Return error instead of panicking if reader buffer is inadequate
* Simplify loop
* Remove unnecessary mut
* Simplify increment
* Improve variable name
* Simplify loop
* Simplify loops
* Improve documentation and simplify loop

src/client/raw_client.rs

@@ -214,13 +214,8 @@ where
         let len = {
             let mut subs = SubscriptionPacket::<'b, TOPICS, MAX_PROPERTIES>::new();
             subs.packet_identifier = identifier;
-            let mut i = 0;
+            for topic_name in topic_names.iter() {
-            loop {
+                subs.add_new_filter(topic_name, self.config.max_subscribe_qos);
-                if i == TOPICS {
-                    break;
-                }
-                subs.add_new_filter(topic_names.get(i).unwrap(), self.config.max_subscribe_qos);
-                i += 1;
             }
             subs.encode(self.buffer, self.buffer_len)
         };
@@ -392,18 +387,13 @@ where
                     return Err(ReasonCode::BuffError);
                 }
                 let (packet_identifier, reasons) = reason.unwrap();
-                let mut i = 0;
+                for reason_code in &reasons {
-                loop {
+                    if *reason_code
-                    if i == reasons.len() {
-                        break;
-                    }
-                    if *reasons.get(i).unwrap()
                         != (<QualityOfService as Into<u8>>::into(self.config.max_subscribe_qos)
                             >> 1)
                     {
-                        return Err(ReasonCode::from(*reasons.get(i).unwrap()));
+                        return Err(ReasonCode::from(*reason_code));
                     }
-                    i += 1;
                 }
                 Ok(Event::Suback(packet_identifier))
             }

src/packet/v5/mqtt_packet.rs

@@ -52,19 +52,12 @@ pub trait Packet<'a> {
         &mut self,
         properties: &Vec<Property<'a>, MAX_PROPERTIES>,
     ) -> u32 {
-        let mut i = 0;
-        let max = properties.len();
         let mut res: u32 = 0;
-        loop {
+        for prop in properties.iter() {
-            let prop = properties.get(i).unwrap();
             if self.property_allowed(prop) {
                 self.push_to_properties((*prop).clone());
                 res = res + prop.encoded_len() as u32 + 1;
             }
-            i += 1;
-            if i == max {
-                break;
-            }
         }
         res
     }
@@ -77,7 +70,7 @@ pub trait Packet<'a> {
     /// Method is decoding Byte array pointing to properties into heapless Vec
     /// in packet. If decoding goes wrong method is returning Error
     fn decode_properties(&mut self, buff_reader: &mut BuffReader<'a>) -> Result<(), BufferError> {
-        self.set_property_len(buff_reader.read_variable_byte_int().unwrap());
+        self.set_property_len(buff_reader.read_variable_byte_int()?);
         let mut x: u32 = 0;
         let mut prop: Property;
         if self.get_property_len() != 0 {

src/tests/unit/packet/v5/connack_packet_unit.rs

@@ -68,7 +68,7 @@ fn test_decode() {
         0x15,
     ];
     let mut connack_res = ConnackPacket::<2>::new();
-    let res = connack_res.decode(&mut BuffReader::new(&mut buffer, 8));
+    let res = connack_res.decode(&mut BuffReader::new(&buffer, 8));
 
     assert!(res.is_ok());
     assert_eq!(connack_res.property_len, 3);

src/utils/buffer_writer.rs

@@ -85,22 +85,13 @@ impl<'a> BuffWriter<'a> {
         }
     }
 
-    /// Writes an array to the buffer.
+    /// Writes (part of) an array to the buffer.
     pub fn insert_ref(&mut self, len: usize, array: &[u8]) -> Result<(), BufferError> {
-        let mut x: usize = 0;
         if self.position + len > self.len {
             return Err(BufferError::InsufficientBufferSize);
         }
-        if len != 0 {
+        self.buffer[self.position..self.position+len].copy_from_slice(&array[0..len]);
-            loop {
+        self.increment_position(len);
-                self.buffer[self.position] = array[x];
-                self.increment_position(1);
-                x += 1;
-                if x == len {
-                    break;
-                }
-            }
-        }
         Ok(())
     }
 
@@ -167,17 +158,8 @@ impl<'a> BuffWriter<'a> {
         &mut self,
         properties: &Vec<Property<'a>, LEN>,
     ) -> Result<(), BufferError> {
-        let mut i = 0;
+        for prop in properties.iter() {
-        let len = properties.len();
+            self.write_property(prop)?;
-        if len != 0 {
-            loop {
-                let prop: &Property = properties.get(i).unwrap_or(&Property::Reserved());
-                self.write_property(prop)?;
-                i += 1;
-                if i == len {
-                    break;
-                }
-            }
         }
         Ok(())
     }
@@ -204,14 +186,8 @@ impl<'a> BuffWriter<'a> {
         len: usize,
         filters: &Vec<TopicFilter<'a>, MAX>,
     ) -> Result<(), BufferError> {
-        let mut i = 0;
+        for filter in filters {
-        loop {
+            self.write_topic_filter_ref(sub, filter)?;
-            let topic_filter: &TopicFilter<'a> = filters.get(i).unwrap();
-            self.write_topic_filter_ref(sub, topic_filter)?;
-            i += 1;
-            if i == len {
-                break;
-            }
         }
         Ok(())
     }

tests/integration_test_single.rs

@@ -66,7 +66,7 @@ async fn publish_core<'b>(
     qos: QualityOfService,
     topic: &str,
     message: &str,
-    err: bool,
+    should_err: bool,
 ) -> Result<(), ReasonCode> {
     info!(
         "[Publisher] Connection to broker with username {} and password {}",
@@ -85,7 +85,7 @@ async fn publish_core<'b>(
         .send_message(topic, message.as_bytes(), qos, false)
         .await;
     info!("[PUBLISHER] sent");
-    if err == true {
+    if should_err {
         assert_err!(result);
     } else {
         assert_ok!(result);

tests/load_test.rs

@@ -83,7 +83,7 @@ async fn publish_core<'b>(
             .await;
         info!("[PUBLISHER] sent {}", count);
         assert_ok!(result);
-        count = count + 1;
+        count += 1;
         if count == amount {
             break;
         }
@@ -150,7 +150,7 @@ async fn receive_core<'b>(
         let act_message = String::from_utf8_lossy(msg?.1);
         info!("[Receiver] Got new {}. message: {}", count, act_message);
         assert_eq!(act_message, MSG);
-        count = count + 1;
+        count += 1;
         if count == amount {
             break;
         }