Summary
Several design issues in hotfix-message affect correctness, API ergonomics, and performance.
Problems
Asymmetric get/set on Message
Message implements Part by delegating get_field_map() to the body, but overrides set() to route fields to the correct part (header/body/trailer) based on FieldLocation. This means message.get(SENDER_COMP_ID) returns Missing even when the field exists in the header, while message.set(SENDER_COMP_ID, "X") correctly routes to the header.
Part trait has too many responsibilities
Part combines field access, mutation, group management, and length calculation. The length calculation semantics differ between parts — Header skips BeginString/BodyLength, Trailer skips CheckSum, Body has no exclusions — but these differences are hidden behind the same interface.
encode() mutates the message
Message::encode() sets BodyLength and CheckSum on self as a side effect. Encoding a message shouldn't require &mut self, and encoding twice may produce unexpected results.
Per-field allocation during parsing
Every parsed field allocates a Vec<u8> via .to_vec(). For high-throughput FIX parsing, a zero-copy approach where fields borrow from the input buffer would avoid allocation on every field.
MessageBuilder naming
MessageBuilder is a parser/factory, not a builder. The name conflicts with the builder pattern and is confusing since actual message construction uses Message::new() + Part::set().
tag_from_bytes doesn't validate input
Non-digit bytes produce garbage results via silent arithmetic overflow rather than returning None.
tags.rs vs session_fields.rs
The same session-level fields (BeginString, BodyLength, MsgType, CheckSum) are defined in both tags.rs and re-exported from session_fields.rs/fixt11, with different modules importing from different sources.
Summary
Several design issues in
hotfix-messageaffect correctness, API ergonomics, and performance.Problems
Asymmetric
get/setonMessageMessageimplementsPartby delegatingget_field_map()to the body, but overridesset()to route fields to the correct part (header/body/trailer) based onFieldLocation. This meansmessage.get(SENDER_COMP_ID)returnsMissingeven when the field exists in the header, whilemessage.set(SENDER_COMP_ID, "X")correctly routes to the header.Parttrait has too many responsibilitiesPartcombines field access, mutation, group management, and length calculation. The length calculation semantics differ between parts —HeaderskipsBeginString/BodyLength,TrailerskipsCheckSum,Bodyhas no exclusions — but these differences are hidden behind the same interface.encode()mutates the messageMessage::encode()setsBodyLengthandCheckSumonselfas a side effect. Encoding a message shouldn't require&mut self, and encoding twice may produce unexpected results.Per-field allocation during parsing
Every parsed field allocates a
Vec<u8>via.to_vec(). For high-throughput FIX parsing, a zero-copy approach where fields borrow from the input buffer would avoid allocation on every field.MessageBuildernamingMessageBuilderis a parser/factory, not a builder. The name conflicts with the builder pattern and is confusing since actual message construction usesMessage::new()+Part::set().tag_from_bytesdoesn't validate inputNon-digit bytes produce garbage results via silent arithmetic overflow rather than returning
None.tags.rsvssession_fields.rsThe same session-level fields (
BeginString,BodyLength,MsgType,CheckSum) are defined in bothtags.rsand re-exported fromsession_fields.rs/fixt11, with different modules importing from different sources.