tasteful refactoring — clarity, dedup, structure#94
Merged
Conversation
identical function defined in both aof.rs and snapshot.rs. now lives in format.rs as a pub fn, both callers import from there.
24 flat constants reorganized into logical sections: string, list, sorted set, hash, set, key lifecycle, protobuf. values unchanged.
ZREM, HDEL, SADD, and SREM all did identical work: read count, alloc vec with capped capacity, loop read_string. shared helper replaces ~7 lines per call site with ~3.
type-tag → SnapValue parsing was duplicated between read_plaintext_entry and read_encrypted_entry. extracted the clean version as parse_snap_value, used in the encrypted path. plaintext path stays inline because it interleaves CRC buffer mirroring.
incr_result() handles the IncrError → ShardResponse pattern (5 call sites), write_result_len() handles WriteError → Len (4 call sites). each call site shrinks from 4-5 lines to 1.
resolve_password(), parse_bind_addr(), and build_persistence_config() pulled out of main() so the startup flow is scannable at a glance. three bind-address parse blocks replaced with a shared helper.
kacy
added a commit
that referenced
this pull request
Feb 19, 2026
* refactor: move capped_capacity to shared format module identical function defined in both aof.rs and snapshot.rs. now lives in format.rs as a pub fn, both callers import from there. * docs: group aof tag constants by data type 24 flat constants reorganized into logical sections: string, list, sorted set, hash, set, key lifecycle, protobuf. values unchanged. * refactor: extract read_string_list helper in aof deserialization ZREM, HDEL, SADD, and SREM all did identical work: read count, alloc vec with capped capacity, loop read_string. shared helper replaces ~7 lines per call site with ~3. * refactor: extract parse_snap_value for encrypted snapshot reading type-tag → SnapValue parsing was duplicated between read_plaintext_entry and read_encrypted_entry. extracted the clean version as parse_snap_value, used in the encrypted path. plaintext path stays inline because it interleaves CRC buffer mirroring. * refactor: extract incr/write error conversion helpers in shard dispatch incr_result() handles the IncrError → ShardResponse pattern (5 call sites), write_result_len() handles WriteError → Len (4 call sites). each call site shrinks from 4-5 lines to 1. * refactor: extract startup helpers from main resolve_password(), parse_bind_addr(), and build_persistence_config() pulled out of main() so the startup flow is scannable at a glance. three bind-address parse blocks replaced with a shared helper.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
summary
after three rounds of hardening, this is purely cosmetic: reducing duplication, improving organization, and making functions scannable. six independent changes, one per commit.
capped_capacitytoformat.rs— identical function lived in bothaof.rsandsnapshot.rs, now shared from the format moduleread_string_listhelper — ZREM, HDEL, SADD, SREM deserialization shared a read-count-then-loop-strings pattern. each call site shrinks from ~7 to ~3 linesparse_snap_value— type-tag → SnapValue parsing duplicated between plaintext and encrypted snapshot paths. extracted the clean version for the encrypted path; plaintext stays inline (CRC buffer mirroring)incr_result/write_result_len— IncrError and WriteError → ShardResponse conversion appeared 9 times identically in dispatch. helper functions replace 4-5 lines per site with 1main.rs—resolve_password(),parse_bind_addr(),build_persistence_config()pulled out so the startup flow reads at a glancewhat was tested
cargo test -p emberkv-core -p ember-persistence -p ember-server -p ember-cluster -p ember-protocol— all passcargo clippy --workspace -- -D warnings— cleancargo fmt --all -- --check— clean--features encryptionbuildsdesign considerations
connection.rs execute()alone — 800 lines but each arm is self-contained and greppable. abstracting would obscure response patternskeyspace.rsentry lookup alone — ~40 occurrences but each has subtly different logic. a generic closure would hide important detailsparse_snap_valuegated behind#[cfg(feature = "encryption")]— only the encrypted read path uses it; plaintext path interleaves CRC buffer writes so can't share the function