Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: jbesraa <[email protected]>
  • Loading branch information
Shourya742 and jbesraa authored Dec 7, 2024
1 parent 216e421 commit ba8b84d
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,18 @@ impl<'a> GetSize for EncodablePrimitive<'a> {

/// The [`EncodableField`] enum defines encodable fields, which may be a primitive or struct.
///
/// Each `EncodableField` represents either a primitive value or a collection of values
/// (a structure). The encoding process for `EncodableField` supports nesting, allowing
/// Each [`EncodableField`] represents either a primitive value or a collection of values
/// (a struct). The encoding process for [`EncodableField`] supports nesting, allowing
/// for complex hierarchical data structures to be serialized.
#[derive(Debug)]
pub enum EncodableField<'a> {
/// Represents an encodablePrimitive
/// Represents a primitive value
///
/// For the full supported list please see [`EncodablePrimitive`]
Primitive(EncodablePrimitive<'a>),
/// Represents a structure of multiple Encodable Field
/// Represents a struct like field structure.
///
/// Note that this is a recursive enum type.
Struct(Vec<EncodableField<'a>>),
}

Expand Down Expand Up @@ -237,10 +241,6 @@ impl<'a> EncodableField<'a> {
}
}

// Provides the logic for calculating the size of the encodable field.
//
// The `get_size` method returns the size in bytes required to encode the field.
// For structucred fields, it calculates the total size of all contained fields.
impl<'a> GetSize for EncodableField<'a> {
fn get_size(&self) -> usize {
match self {
Expand Down
34 changes: 5 additions & 29 deletions protocols/v2/binary-sv2/no-serde-sv2/codec/src/codec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,9 @@ use alloc::vec::Vec;
///
/// It defines two methods for retrieving the size of an encoded message:
///
/// - `size_hint` is a static method that takes the raw data and an offset and returns the total
/// size of the encoded message. This is particularly useful for types where the encoded size may
/// vary based on the contents of the data, and we need to calculate how much space is required
/// for decoding.
/// - `size_hint_` is an instance method that performs the same function but allows the size to be
/// be determined with respect to the current instance of the type.
///
/// These methods are crucial in decoding scenarios where the full size of the message
/// is not immediately known, helping to determine how many bytes need to be read.
/// is not immediately available, helping to determine how many bytes need to be read.
pub trait SizeHint {
/// `size_hint` is a static method that takes the raw data and an offset and returns the total
/// size of the encoded message. This is particularly useful for types where the encoded size
Expand All @@ -91,26 +85,21 @@ pub trait SizeHint {
fn size_hint_(&self, data: &[u8], offset: usize) -> Result<usize, Error>;
}

/// The `GetSize` trait returns the total size in bytes of an encodable type.
///
/// It provides a single method, `get_size`, which returns the total size of the type
/// in bytes.
/// [`GetSize`] is a trait defining a single function that calculates the total size of an encodable type.
pub trait GetSize {
/// `get_size` returns total size of the type in bytes.
fn get_size(&self) -> usize;
}

#[cfg(feature = "with_buffer_pool")]
impl GetSize for Slice {
// Provides the total size of a `Slice` by returning its length.
//
// This implementation for the `Slice` type returns the number of bytes in the
// slice, which represents its total size when encoded.
fn get_size(&self) -> usize {
self.len()
}
}
/// The `Fixed` trait is implemented by all primitives with a constant, fixed size.
/// [`Fixed`] is a trait is defining a single element representing a size of a constant.
///
/// This is useful for primitives with a constant fixed size
///
/// Types implementing this trait must define the constant `SIZE`, representing the
/// fixed number of bytes needed to encode or decode the type. This trait is used for
Expand All @@ -135,29 +124,16 @@ pub trait Variable {
}

impl<T: Fixed> SizeHint for T {
// Provides the size hind for a fixed-size type
//
// Since the type implementing `Fixed` has a constant size, the `size_hint` method
// simply returns the fixed size, making it easy to predict the size of the encoded data.
fn size_hint(_data: &[u8], _offset: usize) -> Result<usize, Error> {
Ok(Self::SIZE)
}

// Instance-based size hint for a fixed-size type.
//
// Similar to the static `size_hint_`, this method returns the constant size for
// the specific instance of the type.
fn size_hint_(&self, _: &[u8], _offset: usize) -> Result<usize, Error> {
Ok(Self::SIZE)
}
}

impl<T: Fixed> GetSize for T {
// Returns the size of the fixed-size type.
//
// As the type implements `Fixed`, this method directly returns the constant `SIZE`
// associated with the type. This is useful when encoding or decoding to know exactly
// how much space the type occupies in memory or when serialized.
fn get_size(&self) -> usize {
Self::SIZE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,11 @@ macro_rules! impl_sv2_for_unsigned {
}
};
}

// Implement Sv2DataType for u8, u16, u32, and u64 using the macro.
impl_sv2_for_unsigned!(u8);
impl_sv2_for_unsigned!(u16);
impl_sv2_for_unsigned!(u32);
impl_sv2_for_unsigned!(u64);

// Implementation of the `Fixed` trait for `f32`.
impl Fixed for f32 {
const SIZE: usize = 4;
}
Expand Down Expand Up @@ -188,7 +185,6 @@ impl U24 {

impl_sv2_for_unsigned!(U24);

// Conversion between `u32` and `U24`.
impl TryFrom<u32> for U24 {
type Error = Error;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ use core::convert::TryInto;
#[cfg(not(feature = "no_std"))]
use std::io::{Error as E, Read, Write};

/// The `Sv2DataType` trait defines methods for encoding and decoding data in the SV2 protocol.
/// `Sv2DataType` is a trait that defines methods for encoding and decoding Stratum V2 data.
/// It is used for serializing and deserializing both fixed-size and dynamically-sized types.
///
/// Key Responsibilities:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,6 @@ impl<'a, const ISFIXED: bool, const SIZE: usize, const HEADERSIZE: usize, const
}
}

// Implementation of tryfrom trait for mutable shared buffer, to convert to Inner<'a, ISFIXED, SIZE,
// HEADERSIZE, MAXSIZE>
impl<'a, const ISFIXED: bool, const SIZE: usize, const HEADERSIZE: usize, const MAXSIZE: usize>
TryFrom<&'a mut [u8]> for Inner<'a, ISFIXED, SIZE, HEADERSIZE, MAXSIZE>
{
Expand Down Expand Up @@ -270,8 +268,6 @@ impl<'a, const ISFIXED: bool, const SIZE: usize, const HEADERSIZE: usize, const
}
}

// Implementation of tryfrom trait for Vec<u8>, to convert to Inner<'a, ISFIXED, SIZE, HEADERSIZE,
// MAXSIZE>
impl<'a, const ISFIXED: bool, const SIZE: usize, const HEADERSIZE: usize, const MAXSIZE: usize>
TryFrom<Vec<u8>> for Inner<'a, ISFIXED, SIZE, HEADERSIZE, MAXSIZE>
{
Expand Down Expand Up @@ -304,8 +300,6 @@ impl<'a, const ISFIXED: bool, const SIZE: usize, const HEADERSIZE: usize, const
}
}

// Implementation of GetSize trait, to get size of type Inner<'a, ISFIXED, SIZE, HEADERSIZE,
// MAXSIZE>
impl<'a, const ISFIXED: bool, const SIZE: usize, const HEADERSIZE: usize, const MAXSIZE: usize>
GetSize for Inner<'a, ISFIXED, SIZE, HEADERSIZE, MAXSIZE>
{
Expand All @@ -317,8 +311,6 @@ impl<'a, const ISFIXED: bool, const SIZE: usize, const HEADERSIZE: usize, const
}
}

// Implementation of GetSize trait, to get expected size of type Inner<'a, ISFIXED, SIZE,
// HEADERSIZE, MAXSIZE>
impl<'a, const ISFIXED: bool, const HEADERSIZE: usize, const SIZE: usize, const MAXSIZE: usize>
SizeHint for Inner<'a, ISFIXED, HEADERSIZE, SIZE, MAXSIZE>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,6 @@ impl<'a> U32AsRef<'a> {
}
}

// Converts a `u32` into a `U32AsRef<'a>`, storing the value as
// a little-endian byte array.
impl<'a> From<u32> for U32AsRef<'a> {
fn from(v: u32) -> Self {
let bytes = v.to_le_bytes();
Expand All @@ -134,7 +132,6 @@ impl<'a> From<u32> for U32AsRef<'a> {
}
}

// Converts a reference to `U32AsRef<'a>` into a `u32`.
impl<'a> From<&'a U32AsRef<'a>> for u32 {
fn from(v: &'a U32AsRef<'a>) -> Self {
let b = v.inner_as_ref();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl<'a, const SIZE: usize> Seq064K<'a, super::inner::Inner<'a, true, SIZE, 0, 0
#[cfg(not(feature = "no_std"))]
use std::io::Read;

/// `Seq0255` represents a sequence of items with a maximum length of 255 elements.
/// [`Seq0255`] represents a sequence with a maximum length of 255 elements.
/// This structure uses a generic type `T` and a lifetime parameter `'a`
/// to ensure compatibility with `serde-sv2`.
#[repr(C)]
Expand Down Expand Up @@ -162,7 +162,7 @@ impl<'a, T: GetSize> GetSize for Seq0255<'a, T> {
}
}

/// `Seq064K` represents a sequence of items with a maximum length of 65535 elements.
/// [`Seq064K`] represents a sequence with a maximum length of 65535 elements.
/// This structure uses a generic type `T` and a lifetime parameter `'a`
/// to ensure compatibility with `serde-sv2`.
#[derive(Debug, Clone, Eq, PartialEq)]
Expand Down Expand Up @@ -196,7 +196,6 @@ impl<'a, T: 'a> Seq064K<'a, T> {
}

impl<'a, T: GetSize> GetSize for Seq064K<'a, T> {
// Calculates the total size of the sequence in bytes.
fn get_size(&self) -> usize {
let mut size = Self::HEADERSIZE;
for with_size in &self.0 {
Expand Down

0 comments on commit ba8b84d

Please sign in to comment.