This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fury.git
The following commit(s) were added to refs/heads/main by this push:
new e37c25e6 fix(dart): replace string-based type selection with enum in
FixedNum factory method (#2185)
e37c25e6 is described below
commit e37c25e625e0bfc0db9251d8fe298eb1e7ba355c
Author: LouShaokun <[email protected]>
AuthorDate: Thu Apr 24 19:17:33 2025 +0800
fix(dart): replace string-based type selection with enum in FixedNum
factory method (#2185)
<!--
**Thanks for contributing to Fury.**
**If this is your first time opening a PR on Fury, you can refer to
[CONTRIBUTING.md](https://github.com/apache/fury/blob/main/CONTRIBUTING.md).**
Contribution Checklist
- The **Apache Fury (incubating)** community has restrictions on the
naming of PR titles. You can also find instructions in
[CONTRIBUTING.md](https://github.com/apache/fury/blob/main/CONTRIBUTING.md).
- Fury has a strong focus on performance. If the PR you submit will have
an impact on performance, please benchmark it first and provide the
benchmark result here.
-->
## What does this PR do?
This PR improves the `FixedNum.from()` factory method by replacing the
string-based type selection with a proper enum (`NumType`). The change
eliminates the risk of runtime errors due to typos in string literals
and enables better IDE support with autocomplete suggestions.
**Before:**
```dart
// String-based type selection
FixedNum.from(42, type: 'int32') // Works
FixedNum.from(42, type: 'INT32') // Works (case insensitive)
FixedNum.from(42, type: 'int-32') // Runtime error
```
**After:**
```dart
// Enum-based type selection
FixedNum.from(42, NumType.int32) // Works, with IDE autocomplete
```
## Related issues
<!--
Is there any related issue? Please attach here.
- #xxxx0
- #xxxx1
- #xxxx2
-->
## Does this PR introduce any user-facing change?
- [x] Does this PR introduce any public API change?
- The `FixedNum.from()` method now takes a `NumType` enum instead of a
string for the type parameter
- The parameter is now positional optional rather than named optional
- [ ] Does this PR introduce any binary protocol compatibility change?
## Benchmark
<!--
When the PR has an impact on performance (if you don't know whether the
PR will have an impact on performance, you can submit the PR first, and
if it will have impact on performance, the code reviewer will explain
it), be sure to attach benchmark data here.
-->
---
.../test/datatype_test/fixed_num_test.dart | 27 ++++++++--------------
.../fury/lib/src/datatype/fury_fixed_num.dart | 19 +++++++--------
2 files changed, 19 insertions(+), 27 deletions(-)
diff --git a/dart/packages/fury-test/test/datatype_test/fixed_num_test.dart
b/dart/packages/fury-test/test/datatype_test/fixed_num_test.dart
index 78aaeda0..de96e963 100644
--- a/dart/packages/fury-test/test/datatype_test/fixed_num_test.dart
+++ b/dart/packages/fury-test/test/datatype_test/fixed_num_test.dart
@@ -20,34 +20,25 @@
// @Skip()
library;
-import 'package:fury/fury.dart' show FixedNum, Float32, Int16, Int32, Int8;
+import 'package:fury/fury.dart' show FixedNum, Float32, Int16, Int32, Int8,
NumType;
import 'package:test/test.dart';
void main() {
group('FixedNum factory & comparability', () {
test('creates correct subtype via FixedNum.from', () {
- expect(FixedNum.from(42, type: 'int8'), isA<Int8>());
- expect(FixedNum.from(42, type: 'int16'), isA<Int16>());
- expect(FixedNum.from(42, type: 'int32'), isA<Int32>());
- expect(FixedNum.from(42, type: 'float32'), isA<Float32>());
+ expect(FixedNum.from(42, NumType.int8), isA<Int8>());
+ expect(FixedNum.from(42, NumType.int16), isA<Int16>());
+ expect(FixedNum.from(42, NumType.int32), isA<Int32>());
+ expect(FixedNum.from(42, NumType.float32), isA<Float32>());
// Default type should be int32
expect(FixedNum.from(42), isA<Int32>());
});
- test('handles case-insensitive type names', () {
- expect(FixedNum.from(42, type: 'INT8'), isA<Int8>());
- expect(FixedNum.from(42, type: 'Int16'), isA<Int16>());
- });
-
- test('throws on unsupported type name', () {
- expect(() => FixedNum.from(42, type: 'invalid'), throwsArgumentError);
- });
-
test('compares values across types', () {
- var a = FixedNum.from(10, type: 'int8');
- var b = FixedNum.from(20, type: 'int8');
- var c = FixedNum.from(10, type: 'int16');
+ var a = FixedNum.from(10, NumType.int8);
+ var b = FixedNum.from(20, NumType.int8);
+ var c = FixedNum.from(10, NumType.int16);
expect(a.compareTo(b) < 0, isTrue);
expect(b.compareTo(a) > 0, isTrue);
@@ -263,7 +254,7 @@ void main() {
var a = Int8(42);
var b = Int8(42);
var c = Int8(43);
- var d = FixedNum.from(42, type: 'int16');
+ var d = FixedNum.from(42, NumType.int16);
expect(a == b, isTrue);
expect(a == c, isFalse);
diff --git a/dart/packages/fury/lib/src/datatype/fury_fixed_num.dart
b/dart/packages/fury/lib/src/datatype/fury_fixed_num.dart
index c0ceb06b..ba8d285e 100644
--- a/dart/packages/fury/lib/src/datatype/fury_fixed_num.dart
+++ b/dart/packages/fury/lib/src/datatype/fury_fixed_num.dart
@@ -23,23 +23,24 @@ import 'int16.dart';
import 'int32.dart';
import 'int8.dart';
+enum NumType { int8, int16, int32, float16, float32 }
+
/// Base abstract class for fixed-size numeric types
abstract base class FixedNum implements Comparable<FixedNum>{
num get value;
// Factory constructor to create the appropriate type
- static FixedNum from(num value, {String type = 'int32'}) {
- switch (type.toLowerCase()) {
- case 'int8': return Int8(value);
- case 'int16': return Int16(value);
- case 'int32': return Int32(value);
- case 'float16': return Float16(value);
- case 'float32': return Float32(value);
- default: throw ArgumentError('Unknown fixed numeric type: $type');
+ static FixedNum from(num value, [NumType type = NumType.int32]) {
+ switch (type) {
+ case NumType.int8: return Int8(value);
+ case NumType.int16: return Int16(value);
+ case NumType.int32: return Int32(value);
+ case NumType.float16: return Float16(value);
+ case NumType.float32: return Float32(value);
}
}
@override
int compareTo(FixedNum other) => value.compareTo(other.value);
-}
\ No newline at end of file
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
