How to optimize code like this？

help
1 
impl Decodable for DataUnit {
    fn decode<D: AsRef<[u8]> + ?Sized>(data: &D) -> Self {
        let data = Vec::from(data.as_ref());
        let len = data.len();
        let data = data.as_slice();
        let mut index = 0;

        let data_type = DataType::decode(data);
        index += data_type.len();
        let data_code = DataCode::decode(&data[index..]);
        index += data_code.len();

        let mut items: Vec<Box<dyn ProtocolDatable>> = Vec::new();
        match data_type {
            DataType::Ack => {
                loop {
                    if index >= len {
                        break;
                    }
                    let item = AckItem::decode(&data[index..]);
                    index += item.len();
                    items.push(Box::new(item));
                }
                Self { data_type, data_code, items, }
            },
            DataType::ParamSet => {
                loop {
                    if index >= len {
                        break;
                    }
                    let item = ParamSetItem::decode(&data[index..]);
                    index += item.len();
                    items.push(Box::new(item));
                }

                Self { data_type, data_code, items, }
            },
            DataType::VehicleCtrl => {
                loop {
                    if index >= len {
                        break;
                    }
                    let item = VehicleCtrlItem::decode(&data[index..]);
                    index += item.len();
                    items.push(Box::new(item));
                }

                Self { data_type, data_code, items, }
            },
            DataType::BlueKey => {
                loop {
                    if index >= len {
                        break;
                    }
                    let item = BlueKeyItem::decode(&data[index..]);
                    index += item.len();
                    items.push(Box::new(item));
                }

                Self { data_type, data_code, items, }
            },
            DataType::BlueKeyV2 => {
                loop {
                    if index >= len {
                        break;
                    }
                    let item = BlueKeyItemV2::decode(&data[index..]);
                    index += item.len();
                    items.push(Box::new(item));
                }

                Self { data_type, data_code, items, }
            },
            DataType::ParamQuery => {
                loop {
                    if index >= len {
                        break;
                    }
                    let item = ParamQueryItem::decode(&data[index..]);
                    index += item.len();
                    items.push(Box::new(item));
                }

                Self { data_type, data_code, items, }
            },
            DataType::DataReport => {
                loop {
                    if index >= len {
                        break;
                    }
                    let item = ReportItem::decode(&data[index..]);
                    index += item.len();
                    items.push(Box::new(item));
                }

                Self { data_type, data_code, items, }
            },
            DataType::FileTrans => {
                loop {
                    if index >= len {
                        break;
                    }
                    let item = FileItem::decode(&data[index..]);
                    index += item.len();
                    items.push(Box::new(item));
                }

                Self { data_type, data_code, items, }
            },
        }
    }
}

There are too many similar duplicate codes, how to optimize them?

2

If nothing else works, probably a macro could do

impl Decodable for DataUnit {
    fn decode<D: AsRef<[u8]> + ?Sized>(data: &D) -> Self {
        let data = Vec::from(data.as_ref());
        let len = data.len();
        let data = data.as_slice();
        let mut index = 0;

        let data_type = DataType::decode(data);
        index += data_type.len();
        let data_code = DataCode::decode(&data[index..]);
        index += data_code.len();

        let mut items: Vec<Box<dyn ProtocolDatable>> = Vec::new();

        macro_rules! match_arm {
            ($Item:ident) => {{
                loop {
                    if index >= len {
                        break;
                    }
                    let item = $Item::decode(&data[index..]);
                    index += item.len();
                    items.push(Box::new(item));
                }
                Self { data_type, data_code, items, }
            }}
        }
        
        match data_type {
            DataType::Ack => match_arm!(AckItem),
            DataType::ParamSet => match_arm!(ParamSetItem),
            DataType::VehicleCtrl => match_arm!(VehicleCtrlItem),
            DataType::BlueKey => match_arm!(BlueKeyItem),
            DataType::BlueKeyV2 => match_arm!(BlueKeyItemV2),
            DataType::ParamQuery => match_arm!(ParamQueryItem),
            DataType::DataReport => match_arm!(ReportItem),
            DataType::FileTrans => match_arm!(FileItem),
        }
    }
}

I think, this can probably be made to work with some fn definition, too (closure would have been my first thought, by those cannot be generic, so taking a type like AckItem/ParamSetItem/... as generic parameter doesn’t work) though it’s harder to test this approach for me without having the types present in a complete example where I could ask the compiler “does it compile yet?”

3

Maybe you could implement a Decode trait with a decode method for every item and have a function like this:

fn from_data<D: Decode>(mut items: Vec<Box<dyn ProtocolDatable>>, data: &[u8], index: usize) -> DataUnit {
    let len = data.len();

    loop {
        if index >= len {
            break;
        }
        let item = D::decode(&data[index..]);
        index += item.len();
        items.push(Box::new(item));
    }

    DataUnit { data_type, data_code, items, }
}
1 Like
4

It has lifecycle problem.
"the parameter type D may not live long enough"
The code is:

pub(self) fn decode_items<T>(data: &[u8], index: &mut usize, len: usize, data_type: DataType, data_code: DataCode) -> DataUnit
    where
        T: Decodable + LengthComputable + ProtocolDatable {
    let mut items: Vec<Box<dyn ProtocolDatable>> = Vec::new();
    loop {
        if *index >= len {
            break;
        }
        let item = T::decode(&data[*index..]);
        *index += item.len();
        items.push(Box::new(item));
    }

    DataUnit { data_type, data_code, items, }
}
5

Without compiler help, this is the best I could come up with

impl Decodable for DataUnit {
    fn decode<D: AsRef<[u8]> + ?Sized>(data: &D) -> Self {
        let data = Vec::from(data.as_ref());
        let len = data.len();
        let data = data.as_slice();
        let mut index = 0;

        let data_type = DataType::decode(data);
        index += data_type.len();
        let data_code = DataCode::decode(&data[index..]);
        index += data_code.len();

        fn match_arm<Item: Decodable + ProtocolDatable + 'static>(mut index: usize, len: usize) -> Vec<Box<dyn ProtocolDatable>> {
            let mut items: Vec<Box<dyn ProtocolDatable>> = Vec::new();
            loop {
                if index >= len {
                    break;
                }
                let item = Item::decode(&data[index..]);
                index += item.len();
                items.push(Box::new(item));
            }
        }
        
        let items = match data_type {
            DataType::Ack => match_arm::<AckItem>(index, len),
            DataType::ParamSet => match_arm::<ParamSetItem>(index, len),
            DataType::VehicleCtrl => match_arm::<VehicleCtrlItem>(index, len),
            DataType::BlueKey => match_arm::<BlueKeyItem>(index, len),
            DataType::BlueKeyV2 => match_arm::<BlueKeyItemV2>(index, len),
            DataType::ParamQuery => match_arm::<ParamQueryItem>(index, len),
            DataType::DataReport => match_arm::<ReportItem>(index, len),
            DataType::FileTrans => match_arm::<FileItem>(index, len),
        }
        Self { data_type, data_code, items, }
    }
}

Edit: Ah, I’ve missed the item.len() call… judging from your later reply, that might be possible with LengthComputable?

Edit2: And I’ve overlooked data, great! That’s why I wish for compiler support with refactors like this.

6

Probably just needs a + 'static in the right place. Creating Box<dyn ProtocolDatable>, which is shorthand for Box<dyn ProtocolDatable + 'static> requires the boxed type to be valid for 'static lifetime.

7

It doesn't work.

8

Maybe you could post a full error message, and a code example that's as complete as possible?

9 
72 |                 items.push(item);
   |                            ^^^^
   |                            |
   |                            the parameter type `T` must be valid for the static lifetime...
   |                            ...so that the type `T` will meet its required lifetime bounds
   |
help: consider adding an explicit lifetime bound
   |
63 |                 T: Decodable + LengthComputable + ProtocolDatable + 'static {
   |                                                                   +++++++++
10

Well, and what if you tried this?

11 
62 |         fn decode_items<T>(data: &[u8], index: &mut usize, len: usize, data_type: DataType, data_code: DataCode) -> DataUnit
   |            ------------ required by a bound in this function
63 |             where
64 |                 T: Decodable + LengthComputable + ProtocolDatable + 'static {
   |                                                   ^^^^^^^^^^^^^^^ required by this bound in `decode_items`
help: consider specifying the generic argument
   |
82 |                 decode_items::<T>(data, &mut index, len, data_type, data_code)

Then:

62 |         fn decode_items<T>(data: &[u8], index: &mut usize, len: usize, data_type: DataType, data_code: DataCode) -> DataUnit
   |                         ^ required by this bound in `decode_items`
help: consider relaxing the implicit `Sized` restriction
   |
64 |                 T: Decodable + LengthComputable + ProtocolDatable + 'static + ?Sized {
   |                                                                             ++++++++

The trait defined:

pub trait ProtocolDatable: Encodable + Decodable + LengthComputable {}
12

The main part of this error message is missing.

13 
82 |                 decode_items::<dyn ProtocolDatable>(data, &mut index, len, data_type, data_code)
   |                                ^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
   |
   = help: the trait `Sized` is not implemented for `dyn ProtocolDatable`
note: required by a bound in `decode_items`

There is no more.

14

Ah, then the call (decode_items::<dyn ProtocolDatable>) is wrong! Can you post the whole function around line 82?

15

image
image1270×714 68.1 KB

16

Try decode_items::<AckItem> in line 82 perhaps :wink:

On an unrelated note, given the supetrait bounds of ProtocolDatable that you shared, the T: Decodable + LengthComputable + ProtocolDatable + 'static in line 64 can probably be shortened to just T: ProtocolDatable + 'static.

17

Yes! It works. Thanks very!

18

What is mean of 'static lifecycle?

19

From the docs, my emphasis:

As a trait bound, it means the type does not contain any non-static references. Eg. the receiver can hold on to the type for as long as they want and it will never become invalid until they drop it.

20

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.