Skip to content

Commit 796d06c

Browse files
committed
refactor(codec): improve error handling best practices
Go: Add MustBuildFrequencies/MustBuildScaledFrequencies following Go standard library conventions (e.g., regexp.MustCompile). Mark original panic functions as Deprecated. Rust: Replace unwrap() with expect() in test code to provide meaningful error messages on failure.
1 parent f3ed661 commit 796d06c

6 files changed

Lines changed: 79 additions & 37 deletions

File tree

algorithms/arithmetic/rust/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,8 @@ mod tests {
240240

241241
#[test]
242242
fn roundtrip_zero_byte_without_confusing_it_with_eof() {
243-
let encoded = encode(&[0x00]).unwrap();
244-
let decoded = decode(&encoded).unwrap();
243+
let encoded = encode(&[0x00]).expect("encode 0x00 should succeed");
244+
let decoded = decode(&encoded).expect("decode should succeed");
245245

246246
assert_eq!(decoded, vec![0x00]);
247247
}

algorithms/range/rust/src/lib.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,8 @@ mod tests {
260260
#[test]
261261
fn roundtrip_empty() {
262262
let data: Vec<u8> = Vec::new();
263-
let enc = encode(&data).unwrap();
264-
let dec = decode(&enc).unwrap();
263+
let enc = encode(&data).expect("encode empty should succeed");
264+
let dec = decode(&enc).expect("decode empty should succeed");
265265
assert_eq!(dec, data);
266266
}
267267

@@ -272,8 +272,8 @@ mod tests {
272272
let mut data = vec![0u8; 10000];
273273
let mut rng = rand::rngs::StdRng::seed_from_u64(1);
274274
rng.fill_bytes(&mut data);
275-
let enc = encode(&data).unwrap();
276-
let dec = decode(&enc).unwrap();
275+
let enc = encode(&data).expect("encode random data should succeed");
276+
let dec = decode(&enc).expect("decode random data should succeed");
277277
assert_eq!(dec, data);
278278
}
279279

algorithms/shared/go/codec/frequency.go

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,17 +175,27 @@ func BuildCumulativeStrict(freq []uint32, zeroTableMessage string) ([]uint32, er
175175
return nil, NewError(KindCorrupt, zeroTableMessage)
176176
}
177177

178-
// BuildFrequencies counts byte frequencies in the input data.
178+
// MustBuildFrequencies counts byte frequencies in the input data.
179179
// The EOF symbol is always set to 1. It panics if a symbol count would exceed
180180
// math.MaxUint32; use BuildFrequenciesChecked to handle that error explicitly.
181-
func BuildFrequencies(data []byte) []uint32 {
181+
//
182+
// This is a convenience function for use in tests and initialization code where
183+
// the input is known to be safe. For production code, prefer BuildFrequenciesChecked.
184+
func MustBuildFrequencies(data []byte) []uint32 {
182185
freq, err := BuildFrequenciesChecked(data)
183186
if err != nil {
184187
panic(err)
185188
}
186189
return freq
187190
}
188191

192+
// BuildFrequencies counts byte frequencies in the input data.
193+
// Deprecated: Use BuildFrequenciesChecked for proper error handling, or MustBuildFrequencies
194+
// if panic behavior is explicitly desired.
195+
func BuildFrequencies(data []byte) []uint32 {
196+
return MustBuildFrequencies(data)
197+
}
198+
189199
// BuildFrequenciesChecked counts byte frequencies in the input data and
190200
// returns an error instead of overflowing if any symbol count would exceed
191201
// math.MaxUint32. The EOF symbol is always set to 1.
@@ -213,18 +223,29 @@ func BuildFrequenciesFromReader(r io.Reader) ([]uint32, error) {
213223
return freq, nil
214224
}
215225

216-
// BuildScaledFrequencies counts byte frequencies and scales them with the same
217-
// semantics as BuildFrequencies followed by ScaleFrequencies. It panics if a
226+
// MustBuildScaledFrequencies counts byte frequencies and scales them with the same
227+
// semantics as MustBuildFrequencies followed by ScaleFrequencies. It panics if a
218228
// symbol count would exceed math.MaxUint32; use BuildScaledFrequenciesChecked
219229
// to handle that error explicitly.
220-
func BuildScaledFrequencies(data []byte, maxTotal uint32) []uint32 {
230+
//
231+
// This is a convenience function for use in tests and initialization code where
232+
// the input is known to be safe. For production code, prefer BuildScaledFrequenciesChecked.
233+
func MustBuildScaledFrequencies(data []byte, maxTotal uint32) []uint32 {
221234
freq, err := BuildScaledFrequenciesChecked(data, maxTotal)
222235
if err != nil {
223236
panic(err)
224237
}
225238
return freq
226239
}
227240

241+
// BuildScaledFrequencies counts byte frequencies and scales them with the same
242+
// semantics as BuildFrequencies followed by ScaleFrequencies.
243+
// Deprecated: Use BuildScaledFrequenciesChecked for proper error handling, or
244+
// MustBuildScaledFrequencies if panic behavior is explicitly desired.
245+
func BuildScaledFrequencies(data []byte, maxTotal uint32) []uint32 {
246+
return MustBuildScaledFrequencies(data, maxTotal)
247+
}
248+
228249
// BuildScaledFrequenciesChecked counts byte frequencies and scales them with
229250
// the same semantics as BuildFrequenciesChecked followed by ScaleFrequencies.
230251
func BuildScaledFrequenciesChecked(data []byte, maxTotal uint32) ([]uint32, error) {

algorithms/shared/go/codec/frequency_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func TestSymbolLimit(t *testing.T) {
4343

4444
func TestBuildFrequencies(t *testing.T) {
4545
data := []byte("aabbbc")
46-
freq := BuildFrequencies(data)
46+
freq := MustBuildFrequencies(data)
4747

4848
if freq['a'] != 2 {
4949
t.Errorf("freq['a'] = %d, want 2", freq['a'])
@@ -60,7 +60,7 @@ func TestBuildFrequencies(t *testing.T) {
6060
}
6161

6262
func TestBuildFrequencies_Empty(t *testing.T) {
63-
freq := BuildFrequencies([]byte{})
63+
freq := MustBuildFrequencies([]byte{})
6464

6565
// All byte frequencies should be 0
6666
for i := 0; i < 256; i++ {
@@ -93,7 +93,7 @@ func TestAccumulateFrequencies_RejectsSymbolOverflow(t *testing.T) {
9393
func TestBuildScaledFrequencies_ClampsTotalAndPreservesEOF(t *testing.T) {
9494
data := append(bytes.Repeat([]byte{'a'}, 12), bytes.Repeat([]byte{'b'}, 6)...)
9595

96-
freq := BuildScaledFrequencies(data, 8)
96+
freq := MustBuildScaledFrequencies(data, 8)
9797

9898
var total uint32
9999
for _, f := range freq {
@@ -113,10 +113,10 @@ func TestBuildScaledFrequencies_ClampsTotalAndPreservesEOF(t *testing.T) {
113113
func TestBuildScaledFrequencies_MatchesScaleFrequenciesSemantics(t *testing.T) {
114114
data := []byte{'a', 'b', 'c', 'd'}
115115

116-
want := BuildFrequencies(data)
116+
want := MustBuildFrequencies(data)
117117
ScaleFrequencies(want, 4)
118118

119-
got := BuildScaledFrequencies(data, 4)
119+
got := MustBuildScaledFrequencies(data, 4)
120120

121121
for i := range want {
122122
if got[i] != want[i] {
@@ -171,7 +171,7 @@ func TestBuildFrequenciesFromReader_MatchesSliceHelper(t *testing.T) {
171171
t.Fatalf("BuildFrequenciesFromReader failed: %v", err)
172172
}
173173

174-
want := BuildFrequencies(data)
174+
want := MustBuildFrequencies(data)
175175
for i := range want {
176176
if got[i] != want[i] {
177177
t.Fatalf("got[%d] = %d, want %d", i, got[i], want[i])
@@ -188,7 +188,7 @@ func TestBuildScaledFrequenciesFromReader_MatchesSliceHelper(t *testing.T) {
188188
t.Fatalf("BuildScaledFrequenciesFromReader failed: %v", err)
189189
}
190190

191-
want := BuildScaledFrequencies(data, 4)
191+
want := MustBuildScaledFrequencies(data, 4)
192192
for i := range want {
193193
if got[i] != want[i] {
194194
t.Fatalf("got[%d] = %d, want %d", i, got[i], want[i])

algorithms/shared/rust/tests/buffer.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ fn encode_buffer_retries_process_transactionally() {
196196
finish_calls: 0,
197197
};
198198

199-
let output = encode_buffer(&mut encoder, b"x").unwrap();
199+
let output = encode_buffer(&mut encoder, b"x").expect("encode_buffer should succeed");
200200

201201
assert_eq!(encoder.process_calls, 2);
202202
assert_eq!(encoder.process_output_lens.len(), 2);
@@ -213,7 +213,7 @@ fn decode_buffer_retries_process_transactionally() {
213213
finish_calls: 0,
214214
};
215215

216-
let output = decode_buffer(&mut decoder, b"x").unwrap();
216+
let output = decode_buffer(&mut decoder, b"x").expect("decode_buffer should succeed");
217217

218218
assert_eq!(decoder.process_calls, 2);
219219
assert_eq!(decoder.process_output_lens.len(), 2);
@@ -229,7 +229,7 @@ fn encode_buffer_retries_finish_transactionally() {
229229
finish_output_lens: Vec::new(),
230230
};
231231

232-
let output = encode_buffer(&mut encoder, b"x").unwrap();
232+
let output = encode_buffer(&mut encoder, b"x").expect("encode_buffer should succeed");
233233

234234
assert_eq!(encoder.finish_calls, 2);
235235
assert_eq!(encoder.finish_output_lens.len(), 2);
@@ -244,7 +244,7 @@ fn decode_buffer_retries_finish_transactionally() {
244244
finish_output_lens: Vec::new(),
245245
};
246246

247-
let output = decode_buffer(&mut decoder, b"x").unwrap();
247+
let output = decode_buffer(&mut decoder, b"x").expect("decode_buffer should succeed");
248248

249249
assert_eq!(decoder.finish_calls, 2);
250250
assert_eq!(decoder.finish_output_lens.len(), 2);

algorithms/shared/rust/tests/lifecycle.rs

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,18 @@ fn test_lifecycle_basic() {
6262
assert_eq!(enc.state(), State::Ready);
6363

6464
// Process should move to Streaming
65-
enc.process(b"test", &mut []).unwrap();
65+
enc.process(b"test", &mut [])
66+
.expect("process should succeed in Ready state");
6667
assert_eq!(enc.state(), State::Streaming);
6768

6869
// Flush should move to Flushing
69-
enc.flush(&mut []).unwrap();
70+
enc.flush(&mut [])
71+
.expect("flush should succeed in Streaming state");
7072
assert_eq!(enc.state(), State::Flushing);
7173

7274
// Finish should move to Finished
73-
enc.finish(&mut []).unwrap();
75+
enc.finish(&mut [])
76+
.expect("finish should succeed in Flushing state");
7477
assert_eq!(enc.state(), State::Finished);
7578

7679
// Post-finish calls should error and move to Error
@@ -123,13 +126,19 @@ fn test_huffman_roundtrip_via_traits() {
123126
let input = b"hello world";
124127
let mut enc = huffman::new_encoder();
125128
let mut output = vec![0u8; 4096];
126-
enc.process(input, &mut output).unwrap();
127-
let written = enc.finish(&mut output).unwrap();
129+
enc.process(input, &mut output)
130+
.expect("huffman process should succeed");
131+
let written = enc
132+
.finish(&mut output)
133+
.expect("huffman finish should succeed");
128134

129135
let mut dec = huffman::new_decoder();
130136
let mut decoded = vec![0u8; 4096];
131-
dec.process(&output[..written], &mut decoded).unwrap();
132-
let dec_written = dec.finish(&mut decoded).unwrap();
137+
dec.process(&output[..written], &mut decoded)
138+
.expect("huffman decode process should succeed");
139+
let dec_written = dec
140+
.finish(&mut decoded)
141+
.expect("huffman decode finish should succeed");
133142

134143
assert_eq!(&decoded[..dec_written], input);
135144
}
@@ -139,13 +148,19 @@ fn test_arithmetic_roundtrip_via_traits() {
139148
let input = b"test data";
140149
let mut enc = arithmetic::new_encoder();
141150
let mut output = vec![0u8; 4096];
142-
enc.process(input, &mut output).unwrap();
143-
let written = enc.finish(&mut output).unwrap();
151+
enc.process(input, &mut output)
152+
.expect("arithmetic process should succeed");
153+
let written = enc
154+
.finish(&mut output)
155+
.expect("arithmetic finish should succeed");
144156

145157
let mut dec = arithmetic::new_decoder();
146158
let mut decoded = vec![0u8; 4096];
147-
dec.process(&output[..written], &mut decoded).unwrap();
148-
let dec_written = dec.finish(&mut decoded).unwrap();
159+
dec.process(&output[..written], &mut decoded)
160+
.expect("arithmetic decode process should succeed");
161+
let dec_written = dec
162+
.finish(&mut decoded)
163+
.expect("arithmetic decode finish should succeed");
149164

150165
assert_eq!(&decoded[..dec_written], input);
151166
}
@@ -155,13 +170,19 @@ fn test_rle_roundtrip_via_traits() {
155170
let input = b"aaabbbccc";
156171
let mut enc = rle::new_encoder();
157172
let mut output = vec![0u8; 4096];
158-
enc.process(input, &mut output).unwrap();
159-
let written = enc.finish(&mut output).unwrap();
173+
enc.process(input, &mut output)
174+
.expect("rle process should succeed");
175+
let written = enc
176+
.finish(&mut output)
177+
.expect("rle finish should succeed");
160178

161179
let mut dec = rle::new_decoder();
162180
let mut decoded = vec![0u8; 4096];
163-
dec.process(&output[..written], &mut decoded).unwrap();
164-
let dec_written = dec.finish(&mut decoded).unwrap();
181+
dec.process(&output[..written], &mut decoded)
182+
.expect("rle decode process should succeed");
183+
let dec_written = dec
184+
.finish(&mut decoded)
185+
.expect("rle decode finish should succeed");
165186

166187
assert_eq!(&decoded[..dec_written], input);
167188
}

0 commit comments

Comments
 (0)