The common package has accumulated helper-level tech debt (duplicate pointer helpers, redundant generic pointer helper, duplicated byte conversion logic, and inconsistent boolean encoding semantics). This makes the package harder to maintain and increases the risk of subtle behavioral inconsistencies.
This issue proposes a focused refactor to consolidate helpers, reduce duplication, and clarify semantics without changing externally visible behavior (except where existing behavior is clearly inconsistent/bug-prone).
Problems Identified
1) Duplicate pointer helpers
We currently maintain multiple type-specific pointer helpers:
Uint8Ptr, Uint16Ptr, Uint32Ptr
Int8Ptr
BoolPtr
StringPtr
Float32Ptr, Float64Ptr
DurationPtr
Additionally, there is already a generic pointer helper:
DataRatePtr[T any](value T) *T
This is redundant and indicates the package is mid-transition.
2) Byte conversion helpers duplicate logic
UintToBytes and IntToBytes implement the same big-endian shift/write loop. Similarly, Float32ToBytes and Float64ToBytes duplicate the same loop pattern after converting to bits.
This increases maintenance burden and can lead to divergence.
3) Boolean encoding semantics are inconsistent
BoolToBytes(value bool, bit uint8) produces a bitmask (1<<bit) in a byte.
BytesToBool(bytes []byte) checks bytes[0] == 0x01.
These semantics only align in the narrow case where bit == 0 and no other bits are present. This inconsistency can cause incorrect decode/encode behavior when bits other than 0 are used.
4) Minor tech debt / readability issues
TimePointer uses a local variable named time which shadows the imported time package (readability issue).
validateFieldValue constructs a new validator.New() on each call (avoidable overhead).
Proposal
A) Introduce a single generic pointer helper and remove redundant helpers
Add:
func Ptr[T any](v T) *T { return &v }
Then migrate call sites and remove:
Uint8Ptr, Uint16Ptr, Uint32Ptr
Int8Ptr
BoolPtr
StringPtr
Float32Ptr, Float64Ptr
DurationPtr
DataRatePtr (redundant once Ptr[T] exists)
B) Consolidate byte conversion logic
Refactor UintToBytes, IntToBytes, Float32ToBytes, Float64ToBytes to reduce duplication. Options:
- Introduce a shared internal helper for big-endian writing loops, or
- Use
encoding/binary where appropriate.
The key requirement is: preserve existing byte ordering and output length behavior.
C) Clarify boolean encoding and decoding semantics
Pick and document one of the following:
- Bit-flag semantics: provide a
BytesToBoolBit([]byte, bit) decoder and use it consistently with BoolToBytes(value, bit), OR
- Byte-as-bool semantics: remove the
bit parameter and encode 0x00 / 0x01, matching BytesToBool.
This should be treated as a correctness fix (not merely style).
D) Minor readability/performance cleanups (optional but recommended)
- Rename the local variable in
TimePointer to avoid shadowing time.
- Reuse a package-level
validator.Validate instance instead of constructing a new one per field.
Acceptance Criteria
Notes / Non-Goals
- This is not a redesign of the
Decode/Encode reflection model.
- Focus is on helper simplification and reducing maintenance overhead while keeping behavior stable (except for boolean semantics, which currently appear inconsistent).
The
commonpackage has accumulated helper-level tech debt (duplicate pointer helpers, redundant generic pointer helper, duplicated byte conversion logic, and inconsistent boolean encoding semantics). This makes the package harder to maintain and increases the risk of subtle behavioral inconsistencies.This issue proposes a focused refactor to consolidate helpers, reduce duplication, and clarify semantics without changing externally visible behavior (except where existing behavior is clearly inconsistent/bug-prone).
Problems Identified
1) Duplicate pointer helpers
We currently maintain multiple type-specific pointer helpers:
Uint8Ptr,Uint16Ptr,Uint32PtrInt8PtrBoolPtrStringPtrFloat32Ptr,Float64PtrDurationPtrAdditionally, there is already a generic pointer helper:
DataRatePtr[T any](value T) *TThis is redundant and indicates the package is mid-transition.
2) Byte conversion helpers duplicate logic
UintToBytesandIntToBytesimplement the same big-endian shift/write loop. Similarly,Float32ToBytesandFloat64ToBytesduplicate the same loop pattern after converting to bits.This increases maintenance burden and can lead to divergence.
3) Boolean encoding semantics are inconsistent
BoolToBytes(value bool, bit uint8)produces a bitmask (1<<bit) in a byte.BytesToBool(bytes []byte)checksbytes[0] == 0x01.These semantics only align in the narrow case where
bit == 0and no other bits are present. This inconsistency can cause incorrect decode/encode behavior when bits other than0are used.4) Minor tech debt / readability issues
TimePointeruses a local variable namedtimewhich shadows the importedtimepackage (readability issue).validateFieldValueconstructs a newvalidator.New()on each call (avoidable overhead).Proposal
A) Introduce a single generic pointer helper and remove redundant helpers
Add:
Then migrate call sites and remove:
Uint8Ptr,Uint16Ptr,Uint32PtrInt8PtrBoolPtrStringPtrFloat32Ptr,Float64PtrDurationPtrDataRatePtr(redundant oncePtr[T]exists)B) Consolidate byte conversion logic
Refactor
UintToBytes,IntToBytes,Float32ToBytes,Float64ToBytesto reduce duplication. Options:encoding/binarywhere appropriate.The key requirement is: preserve existing byte ordering and output length behavior.
C) Clarify boolean encoding and decoding semantics
Pick and document one of the following:
BytesToBoolBit([]byte, bit)decoder and use it consistently withBoolToBytes(value, bit), ORbitparameter and encode0x00/0x01, matchingBytesToBool.This should be treated as a correctness fix (not merely style).
D) Minor readability/performance cleanups (optional but recommended)
TimePointerto avoid shadowingtime.validator.Validateinstance instead of constructing a new one per field.Acceptance Criteria
Ptr[T any]helperDataRatePtrand all type-specific*Ptrhelpers; migrate all call sitesBoolToBytesvsBytesToBoolsemantic mismatch and update call sites accordinglytimepackage shadowing inTimePointervalidator.New()per callNotes / Non-Goals
Decode/Encodereflection model.