rust package 0.1.1 #221
rust package 0.1.1 #221
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthroughこのプルリクエストでは、 Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
🧹 Outside diff range and nitpick comments (13)
package/umt_rust/doc/index.md (1)
3-3: 冗長な表現を改善してください「personally」という表現は冗長です。以下のように簡潔な表現に修正することを提案します：
- A collection of useful functions that I personally created. + A collection of useful functions that I created.🧰 Tools
🪛 LanguageTool
[style] ~3-~3: This phrase is often redundant. Consider writing “I” unless you want to emphasize that you and not someone else will do something. Then, commas might be missing.
Context: ... A collection of useful functions that I personally created. It is for personal use, so th...(I_MYSELF)
package/umt_rust/tests/math/test_value_swap.rs (1)
3-6: テストケースの拡充が推奨されます現在のテストは基本的な機能のみを確認しています。以下のようなテストケースの追加を検討してください：
- 同じ値を入力した場合のテスト
- 負の数値のテスト
- 極端に大きな値や小さな値のテスト
- 特殊な値（NaN、Infinity）のテスト
以下のようなテストケースの追加を提案します：
#[test] fn test_value_swap_comprehensive() { // 基本的なケース assert_eq!(umt_value_swap(1.0, 2.0), vec![2.0, 1.0]); // 同じ値 assert_eq!(umt_value_swap(1.0, 1.0), vec![1.0, 1.0]); // 負の数 assert_eq!(umt_value_swap(-1.0, -2.0), vec![-2.0, -1.0]); // ゼロを含むケース assert_eq!(umt_value_swap(0.0, 1.0), vec![1.0, 0.0]); }package/umt_rust/src/math/deviation_value.rs (1)
1-1: 関数にドキュメンテーションコメントを追加することを推奨します関数の目的、パラメータの説明、戻り値の説明を含むドキュメンテーションコメントを追加することで、APIの使用者が関数の使い方を理解しやすくなります。
+/// 偏差値を計算します +/// +/// # 引数 +/// * `value` - 対象となる値 +/// * `average_value` - 平均値 +/// * `standard_deviation_value` - 標準偏差 +/// +/// # 戻り値 +/// 計算された偏差値（50を中心とした標準化得点） +/// +/// # パニック +/// 標準偏差が0の場合にパニックします pub fn umt_deviation_value(value: f64, average_value: f64, standard_deviation_value: f64) -> f64 {package/umt_rust/tests/math/test_deviation_value.rs (1)
3-7: テストケースの追加を推奨します現在のテストは基本的なケースのみをカバーしています。以下のようなテストケースの追加を推奨します：
- 負の値のテスト
- 境界値のテスト（極端に大きい値、小さい値）
- average_valueがvalueより大きい場合のテスト
#[test] fn test_deviation_value() { assert_eq!(umt_deviation_value(100.0, 50.0, 10.0), 100.0); assert_eq!(umt_deviation_value(100.0, 50.0, 20.0), 75.0); + // 負の値のテスト + assert_eq!(umt_deviation_value(-50.0, 0.0, 10.0), 0.0); + // 平均値が対象値より大きい場合 + assert_eq!(umt_deviation_value(0.0, 50.0, 10.0), 0.0); + // 極端な値のテスト + assert!(!umt_deviation_value(f64::MAX, 0.0, 1.0).is_infinite()); }package/umt_rust/src/math/linear_congruential_generator.rs (2)
1-11: ジェネリック型の使用が過剰である可能性があります現在の実装では3つのジェネリック型パラメータを使用していますが、これは必要以上に複雑かもしれません。代わりに以下のようなシンプルな実装を検討してください：
-pub fn umt_linear_congruential_generator<M, L, I>( - seed: i32, - max: M, - multiplier: L, - increment: I, -) -> i32 -where - M: Into<Option<i32>>, - L: Into<Option<i32>>, - I: Into<Option<i32>>, +pub fn umt_linear_congruential_generator( + seed: i32, + max: Option<i32>, + multiplier: Option<i32>, + increment: Option<i32>, +) -> i32
1-17: ドキュメントとテストが不足しています関数の使用方法、パラメータの意味、戻り値の範囲などを説明するドキュメントが必要です。また、テストケースも追加する必要があります。
以下のようなドキュメントの追加を提案します：
/// 線形合同法による疑似乱数生成器 /// /// # 引数 /// * `seed` - 乱数生成の初期値 /// * `max` - 生成される乱数の最大値（デフォルト: 2_147_483_647） /// * `multiplier` - 乗数（デフォルト: 1_664_525） /// * `increment` - 増分（デフォルト: 1_013_904_223） /// /// # 戻り値 /// 0からmax-1までの範囲の疑似乱数 /// /// # パニック /// * maxが0以下の場合 /// * multiplierが0以下の場合テストケースの実装をお手伝いしましょうか？
package/umt_rust/tests/math/test_factorize.rs (1)
4-8: テストケースの拡充が必要です現在のテストケースは基本的な機能のみをカバーしています。以下のようなケースも追加することを推奨します：
- 負の数の処理
- より大きな数値（例：100, 1000）
- エッジケース（0など）
- 素数の処理
また、各テストケースの目的を説明するドキュメントコメントの追加も推奨します。
#[test] fn test_factorize() { + // 基本的なケース assert_eq!(umt_factorize(1), vec![1]); assert_eq!(umt_factorize(2), vec![2]); assert_eq!(umt_factorize(10), vec![2, 5]); + + // エッジケース + assert_eq!(umt_factorize(0), vec![]); + + // 大きな数値 + assert_eq!(umt_factorize(100), vec![2, 2, 5, 5]); + + // 素数 + assert_eq!(umt_factorize(17), vec![17]); }package/umt_rust/src/math/factorize.rs (1)
1-1: 関数のドキュメンテーションが必要ですこの公開関数に対して、以下の情報を含むドキュメントコメントを追加することを推奨します：
- 関数の目的
- パラメータの説明
- 戻り値の説明
- 使用例
+/// 与えられた数値を素因数分解します。 +/// +/// # Arguments +/// +/// * `n` - 素因数分解する整数値 +/// +/// # Returns +/// +/// 素因数を含むベクトル +/// +/// # Examples +/// +/// ``` +/// use umt_rust::math::umt_factorize; +/// +/// assert_eq!(umt_factorize(10), vec![2, 5]); +/// ``` pub fn umt_factorize(n: i32) -> Vec<i32> {package/umt_rust/tests/math/test_round_of.rs (1)
4-10: テストケースの拡充が推奨されます現在のテストケースは基本的なケースをカバーしていますが、以下のようなケースの追加を推奨します：
- 極端に大きな数値
- 非常に小さな数値（精度の限界に近い値）
- 無限大やNaNの処理
- 負の精度値に対するパニックテスト
以下のようなテストケースの追加を提案します：
#[test] fn test_round_of_edge_cases() { assert_eq!(umt_round_of(f64::MAX, 2), f64::MAX); assert_eq!(umt_round_of(f64::MIN_POSITIVE, 2), 0.0); assert!(umt_round_of(f64::INFINITY, 2).is_infinite()); assert!(umt_round_of(f64::NAN, 2).is_nan()); } #[test] #[should_panic(expected = "Precision must be non-negative")] fn test_round_of_negative_precision() { umt_round_of(1.23, -1); }package/umt_rust/tests/math/test_softmax.rs (1)
4-20: テストカバレッジを向上させるための追加テストケースの提案より堅牢なテストスイートにするために、以下のようなエッジケースの追加を推奨します：
- 負の数を含むベクトル
- 非常に大きな数値
- 非常に小さな数値
- 空のベクトル
- 単一要素のベクトル
以下のようなテストケースの追加を提案します：
#[test] fn test_softmax() { + // 空のベクトルのテスト + assert_eq!(umt_softmax(vec![], 3), vec![]); + + // 単一要素のテスト + assert_eq!(umt_softmax(vec![1.0], 3), vec![1.000]); + + // 負の数を含むテスト + assert_eq!(umt_softmax(vec![-1.0, 0.0, 1.0], 3), vec![ + 0.090, 0.245, 0.665 + ]); + + // 大きな数値のテスト + assert_eq!(umt_softmax(vec![1000.0, 2000.0, 3000.0], 3), vec![ + 0.000, 0.000, 1.000 + ]); + // 既存のテストケース... }package/umt_rust/tests/math/test_npr.rs (1)
3-6: テストケースの追加を推奨現在のテストケースは基本的な機能のみを確認しています。以下のような追加のテストケースの実装を検討してください：
- エッジケース（n < r の場合）
- 境界値（n = r の場合）
- より大きな数値でのテスト
- 0を含むケース
以下のようなテストケースの追加を提案します：
#[test] fn test_npr() { assert_eq!(umt_npr(7, 3), 210); + assert_eq!(umt_npr(5, 5), 120); // n = r + assert_eq!(umt_npr(4, 0), 1); // r = 0 + assert_eq!(umt_npr(10, 2), 90); // 大きな数値 } + +#[test] +#[should_panic] +fn test_npr_invalid() { + umt_npr(3, 4); // n < r のケース +}package/umt_rust/tests/array/test_range.rs (1)
4-8: テストケースの追加を推奨現在のテストケースは基本的な機能を確認していますが、以下のケースを追加することを推奨します：
- 無効な入力の処理（max < min）
- 大きな範囲のテスト
- 境界値のテスト（i32の最大値、最小値付近）
#[test] fn test_umt_range_edge_cases() { // 無効な入力 assert_eq!(umt_range(5, 1), vec![]); // 大きな範囲 assert_eq!(umt_range(0, 1000).len(), 1000); // 境界値 assert_eq!(umt_range(i32::MAX - 2, i32::MAX), vec![i32::MAX - 2, i32::MAX - 1]); }package/umt_rust/tests/math/test_ncr.rs (1)
1-36: テストカバレッジの改善提案以下のテストケースの追加を推奨します：
- 大きな数値の組み合わせ（例：20C10）
- オーバーフローの可能性がある値の検証
以下のようなテストケースの追加を提案します：
#[test] fn test_ncr_large_numbers() { assert_eq!(umt_ncr(20, 10), 184756); } #[test] fn test_ncr_potential_overflow() { // 具体的な値は使用している数値型に依存します let result = umt_ncr(30, 15); assert!(result > 0); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
package/umt_rust/Cargo.lockis excluded by!**/*.lock,!**/*.lockpackage/umt_rust/Cargo.tomlis excluded by!**/*.tomlpackage/umt_rust/rustfmt.tomlis excluded by!**/*.toml
📒 Files selected for processing (37)
package/umt_rust/.gitignore(1 hunks)package/umt_rust/LICENSE(1 hunks)package/umt_rust/doc/index.md(1 hunks)package/umt_rust/src/array/mod.rs(1 hunks)package/umt_rust/src/array/range.rs(1 hunks)package/umt_rust/src/lib.rs(1 hunks)package/umt_rust/src/math/average.rs(1 hunks)package/umt_rust/src/math/deg_to_rad.rs(1 hunks)package/umt_rust/src/math/deviation_value.rs(1 hunks)package/umt_rust/src/math/factorial.rs(1 hunks)package/umt_rust/src/math/factorize.rs(1 hunks)package/umt_rust/src/math/gcd.rs(1 hunks)package/umt_rust/src/math/get_decimal_length.rs(1 hunks)package/umt_rust/src/math/lcm.rs(1 hunks)package/umt_rust/src/math/linear_congruential_generator.rs(1 hunks)package/umt_rust/src/math/math_separator.rs(1 hunks)package/umt_rust/src/math/mod.rs(1 hunks)package/umt_rust/src/math/ncr.rs(1 hunks)package/umt_rust/src/math/npr.rs(1 hunks)package/umt_rust/src/math/round_of.rs(1 hunks)package/umt_rust/src/math/softmax.rs(1 hunks)package/umt_rust/src/math/value_swap.rs(1 hunks)package/umt_rust/tests/array/test_range.rs(1 hunks)package/umt_rust/tests/math/test_average.rs(1 hunks)package/umt_rust/tests/math/test_deg_to_rad.rs(1 hunks)package/umt_rust/tests/math/test_deviation_value.rs(1 hunks)package/umt_rust/tests/math/test_factorial.rs(1 hunks)package/umt_rust/tests/math/test_factorize.rs(1 hunks)package/umt_rust/tests/math/test_gcd.rs(1 hunks)package/umt_rust/tests/math/test_get_decimal_length.rs(1 hunks)package/umt_rust/tests/math/test_math_separator.rs(1 hunks)package/umt_rust/tests/math/test_ncr.rs(1 hunks)package/umt_rust/tests/math/test_npr.rs(1 hunks)package/umt_rust/tests/math/test_round_of.rs(1 hunks)package/umt_rust/tests/math/test_softmax.rs(1 hunks)package/umt_rust/tests/math/test_value_swap.rs(1 hunks)package/umt_rust/tests/tests.rs(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- package/umt_rust/src/array/mod.rs
- package/umt_rust/LICENSE
- package/umt_rust/.gitignore
- package/umt_rust/tests/tests.rs
- package/umt_rust/src/math/mod.rs
🧰 Additional context used
🪛 LanguageTool
package/umt_rust/doc/index.md
[style] ~3-~3: This phrase is often redundant. Consider writing “I” unless you want to emphasize that you and not someone else will do something. Then, commas might be missing.
Context: ... A collection of useful functions that I personally created. It is for personal use, so th...
(I_MYSELF)
🔇 Additional comments (9)
package/umt_rust/src/math/linear_congruential_generator.rs (1)
1-17: 実装の検証が必要です
この実装が他の数学関数と適切に連携できることを確認する必要があります。
✅ Verification successful
Let's gather more information about potential mathematical dependencies and usage patterns.
Let's check if there are any tests or usage examples for this function.
線形合同法の実装は独立しており、問題ありません
この関数は他の数学関数との依存関係がなく、独立して動作する実装となっています。モジュールとして適切にエクスポートされており、他の数学関数からも呼び出し可能な状態です。追加の検証は不要です。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 他の数学関数との依存関係を確認
rg -l "umt_linear_congruential_generator" package/umt_rust/src/
Length of output: 122
Script:
#!/bin/bash
# Check for any mathematical operations or imports in the same directory
rg -A 5 "use" package/umt_rust/src/math/
Length of output: 3257
Script:
#!/bin/bash
# Check for tests and usage examples
rg -g "*.rs" "linear_congruential_generator" package/umt_rust/
Length of output: 319
package/umt_rust/tests/math/test_softmax.rs (1)
1-20: 実装は正しく、テストケースも適切に書かれています。
現在のテストケースは基本的なシナリオをカバーしており、良い実装です。
package/umt_rust/src/lib.rs (1)
1-2: モジュール構造が適切です！
配列操作と数学的機能を別々のモジュールに分離することで、コードの整理と保守性が向上しています。
package/umt_rust/tests/math/test_ncr.rs (6)
1-1: インポート文は適切です
数学モジュールからumt_ncr関数を正しくインポートしています。
3-6: 基本的な組み合わせのテストケースは適切です
7C3 = 35の計算が正しく検証されています。
8-11: ゼロの組み合わせのテストケースは適切です
0C0 = 1のエッジケースが正しく検証されています。
13-16: nとrが等しい場合のテストケースは適切です
5C5 = 1のケースが正しく検証されています。
18-21: nがrより小さい場合のテストケースは適切です
3C5 = 0の無効なケースが正しく検証されています。
23-26: rがゼロの場合のテストケースは適切です
5C0 = 1のエッジケースが正しく検証されています。
| # UMT Rust | ||
|
|
||
| A collection of useful functions that I personally created. | ||
|
|
||
| It is for personal use, so there may be destructive changes. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ドキュメントの充実化が必要です
現在のドキュメントは非常に簡潔すぎるように見受けられます。以下の項目を追加することを推奨します：
- パッケージの主な機能の概要
- 各モジュール（array、math等）の説明
- インストール方法
- 基本的な使用例
- 破壊的変更が発生する可能性がある具体的な説明
以下のような構造を提案します：
# UMT Rust
Rustで実装された数学的機能を提供するユーティリティパッケージです。
## 機能
- 配列操作（array）
- 範囲生成（range）
- 数学関数（math）
- 平均値計算
- 度数からラジアンへの変換
- 偏差値計算
など
## インストール方法
```cargo.toml
[dependencies]
umt_rust = "0.1.1"使用例
use umt_rust::array::range::umt_range;
fn main() {
let numbers = umt_range(0, 10);
println!("{:?}", numbers);
}注意事項
本パッケージは個人利用を目的として開発されており、以下の変更が予告なく行われる可能性があります：
- APIの変更
- 機能の追加・削除
- 動作の変更
ドキュメントの改善を支援させていただきますが、いかがでしょうか？
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 LanguageTool</summary>
[style] ~3-~3: This phrase is often redundant. Consider writing “I” unless you want to emphasize that you and not someone else will do something. Then, commas might be missing.
Context: ... A collection of useful functions that I personally created. It is for personal use, so th...
(I_MYSELF)
</details>
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| pub fn umt_value_swap(x: f64, y: f64) -> Vec<f64> { | ||
| let mut x = x; | ||
| let mut y = y; | ||
| let temp = x; | ||
| x = y; | ||
| y = temp; | ||
| vec![x, y] | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
実装の最適化とドキュメントの追加を推奨
現在の実装には以下の改善点があります：
Vec<f64>の代わりに(f64, f64)を返すことで、不要なヒープアロケーションを避けることができます- 関数のドキュメントが不足しています
以下のような実装を提案します：
/// 2つの浮動小数点数の値を交換します。
///
/// # 引数
/// * `x` - 1番目の値
/// * `y` - 2番目の値
///
/// # 戻り値
/// 交換後の値のタプル。(y, x)の形式で返します。
///
/// # 例
/// ```
/// let (new_x, new_y) = umt_value_swap(1.0, 2.0);
/// assert_eq!(new_x, 2.0);
/// assert_eq!(new_y, 1.0);
/// ```
pub fn umt_value_swap(x: f64, y: f64) -> (f64, f64) {
(y, x)
}この実装の利点：
- メモリ効率が向上します
- より明確なAPIを提供します
- 適切なドキュメントが含まれています
| pub fn umt_deviation_value(value: f64, average_value: f64, standard_deviation_value: f64) -> f64 { | ||
| (value - average_value) / standard_deviation_value * 10.0 + 50.0 | ||
| } |
There was a problem hiding this comment.
標準偏差が0の場合のエラーハンドリングが必要です
標準偏差値が0の場合、除算でパニックが発生する可能性があります。以下のような対策を推奨します：
pub fn umt_deviation_value(value: f64, average_value: f64, standard_deviation_value: f64) -> f64 {
+ if standard_deviation_value == 0.0 {
+ panic!("標準偏差は0以外である必要があります");
+ }
(value - average_value) / standard_deviation_value * 10.0 + 50.0
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn umt_deviation_value(value: f64, average_value: f64, standard_deviation_value: f64) -> f64 { | |
| (value - average_value) / standard_deviation_value * 10.0 + 50.0 | |
| } | |
| pub fn umt_deviation_value(value: f64, average_value: f64, standard_deviation_value: f64) -> f64 { | |
| if standard_deviation_value == 0.0 { | |
| panic!("標準偏差は0以外である必要があります"); | |
| } | |
| (value - average_value) / standard_deviation_value * 10.0 + 50.0 | |
| } |
| let max = max.into().unwrap_or(2_147_483_647); | ||
| let multiplier = multiplier.into().unwrap_or(1_664_525); | ||
| let increment = increment.into().unwrap_or(1_013_904_223); | ||
|
|
||
| (multiplier * seed + increment) % max |
There was a problem hiding this comment.
整数オーバーフローの可能性があります
乗算操作でオーバーフローが発生する可能性があります。また、入力値の検証も必要です。
以下の修正を提案します：
+ if max <= 0 {
+ panic!("max must be positive");
+ }
+ if multiplier <= 0 {
+ panic!("multiplier must be positive");
+ }
+
- (multiplier * seed + increment) % max
+ ((multiplier as i64 * seed as i64 + increment as i64) % max as i64) as i32📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let max = max.into().unwrap_or(2_147_483_647); | |
| let multiplier = multiplier.into().unwrap_or(1_664_525); | |
| let increment = increment.into().unwrap_or(1_013_904_223); | |
| (multiplier * seed + increment) % max | |
| let max = max.into().unwrap_or(2_147_483_647); | |
| let multiplier = multiplier.into().unwrap_or(1_664_525); | |
| let increment = increment.into().unwrap_or(1_013_904_223); | |
| if max <= 0 { | |
| panic!("max must be positive"); | |
| } | |
| if multiplier <= 0 { | |
| panic!("multiplier must be positive"); | |
| } | |
| ((multiplier as i64 * seed as i64 + increment as i64) % max as i64) as i32 |
| pub fn umt_math_separator(x: i32) -> Vec<i32> { | ||
| let n: i32 = (x.to_string().len() - 1).try_into().unwrap(); | ||
| if n > 0 { | ||
| return vec![ | ||
| 10_i32.pow(n.try_into().unwrap()), | ||
| (x - (10_i32.pow(n.try_into().unwrap()))), | ||
| ]; | ||
| } | ||
| vec![0, x] | ||
| } |
There was a problem hiding this comment.
エラー処理とパフォーマンスの改善が必要です
以下の問題点が見つかりました：
unwrap()の使用は安全ではありません。パニックを避けるためResultを返すべきです。- 負の数の処理が考慮されていません。
- 文字列変換を使用した桁数計算は効率的ではありません。
以下の改善を提案します：
-pub fn umt_math_separator(x: i32) -> Vec<i32> {
- let n: i32 = (x.to_string().len() - 1).try_into().unwrap();
- if n > 0 {
- return vec![
- 10_i32.pow(n.try_into().unwrap()),
- (x - (10_i32.pow(n.try_into().unwrap()))),
- ];
- }
- vec![0, x]
+pub fn umt_math_separator(x: i32) -> Result<Vec<i32>, &'static str> {
+ if x < 0 {
+ return Err("負の数は対応していません");
+ }
+
+ if x < 10 {
+ return Ok(vec![0, x]);
+ }
+
+ let mut n = 0;
+ let mut temp = x;
+ while temp >= 10 {
+ temp /= 10;
+ n += 1;
+ }
+
+ let power = 10_i32.pow(n);
+ Ok(vec![power, x - power])
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn umt_math_separator(x: i32) -> Vec<i32> { | |
| let n: i32 = (x.to_string().len() - 1).try_into().unwrap(); | |
| if n > 0 { | |
| return vec![ | |
| 10_i32.pow(n.try_into().unwrap()), | |
| (x - (10_i32.pow(n.try_into().unwrap()))), | |
| ]; | |
| } | |
| vec![0, x] | |
| } | |
| pub fn umt_math_separator(x: i32) -> Result<Vec<i32>, &'static str> { | |
| if x < 0 { | |
| return Err("負の数は対応していません"); | |
| } | |
| if x < 10 { | |
| return Ok(vec![0, x]); | |
| } | |
| let mut n = 0; | |
| let mut temp = x; | |
| while temp >= 10 { | |
| temp /= 10; | |
| n += 1; | |
| } | |
| let power = 10_i32.pow(n); | |
| Ok(vec![power, x - power]) | |
| } |
| pub fn umt_factorial(mut x: i32) -> i32 { | ||
| let mut result = 1; | ||
| if x != 0 { | ||
| while x > 1 { | ||
| result *= x; | ||
| x -= 1; | ||
| } | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
重要な改善が必要です
現在の実装には以下の問題点があります：
- オーバーフローの危険性（i32は小さな階乗でもオーバーフローします）
- 負の数の入力に対する処理が欠如
- ドキュメンテーションの不足
以下の修正を提案します：
+/// 与えられた非負整数の階乗を計算します。
+///
+/// # Arguments
+/// * `x` - 計算する数値（非負整数）
+///
+/// # Returns
+/// * 階乗の結果
+///
+/// # Panics
+/// * 負の数が入力された場合
+/// * 結果がi64の範囲を超える場合
-pub fn umt_factorial(mut x: i32) -> i32 {
+pub fn umt_factorial(mut x: i32) -> i64 {
+ if x < 0 {
+ panic!("Factorial is not defined for negative numbers");
+ }
- let mut result = 1;
+ let mut result: i64 = 1;
if x != 0 {
while x > 1 {
- result *= x;
+ result = result.checked_mul(x as i64)
+ .expect("Factorial overflow");
x -= 1;
}
}
- return result;
+ result
}また、大きな数値のためのBigIntの使用も検討することを推奨します。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn umt_factorial(mut x: i32) -> i32 { | |
| let mut result = 1; | |
| if x != 0 { | |
| while x > 1 { | |
| result *= x; | |
| x -= 1; | |
| } | |
| } | |
| return result; | |
| } | |
| /// 与えられた非負整数の階乗を計算します。 | |
| /// | |
| /// # Arguments | |
| /// * `x` - 計算する数値（非負整数） | |
| /// | |
| /// # Returns | |
| /// * 階乗の結果 | |
| /// | |
| /// # Panics | |
| /// * 負の数が入力された場合 | |
| /// * 結果がi64の範囲を超える場合 | |
| pub fn umt_factorial(mut x: i32) -> i64 { | |
| if x < 0 { | |
| panic!("Factorial is not defined for negative numbers"); | |
| } | |
| let mut result: i64 = 1; | |
| if x != 0 { | |
| while x > 1 { | |
| result = result.checked_mul(x as i64) | |
| .expect("Factorial overflow"); | |
| x -= 1; | |
| } | |
| } | |
| result | |
| } |
| fn test_factorial() { | ||
| assert_eq!(umt_factorial(0), 1); | ||
| assert_eq!(umt_factorial(1), 1); | ||
| assert_eq!(umt_factorial(2), 2); | ||
| assert_eq!(umt_factorial(3), 6); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
階乗テストの拡充が必要
現在のテストは基本的なケースのみをカバーしています。以下の改善を提案します：
- より大きな数値のテスト
- オーバーフローの処理確認
- テストケースの整理（test_utilsの使用）
#[test]
fn test_factorial_extended() {
// より大きな数値
assert_eq!(umt_factorial(5), 120);
assert_eq!(umt_factorial(10), 3628800);
// オーバーフローの確認
#[should_panic(expected = "overflow")]
#[test]
fn test_factorial_overflow() {
umt_factorial(100);
}
}| fn test_deg_to_rad() { | ||
| assert_eq!(umt_deg_to_rad(0.0), 0.0); | ||
| assert_eq!(umt_deg_to_rad(90.0), 1.5707963267948966); | ||
| assert_eq!(umt_deg_to_rad(180.0), 3.141592653589793); | ||
| assert_eq!(umt_deg_to_rad(270.0), 4.71238898038469); | ||
| assert_eq!(umt_deg_to_rad(360.0), 6.283185307179586); | ||
| } |
There was a problem hiding this comment.
浮動小数点数の比較方法と網羅性の改善
以下の改善点を提案します：
- 浮動小数点数の比較には
assert_eq!の代わりにassert!((x - y).abs() < epsilon)を使用 - 負の角度のテスト追加
- 特殊なケース（NaN、Infinity）の処理確認
#[test]
fn test_deg_to_rad_improved() {
const EPSILON: f64 = 1e-10;
// イプシロンを使用した比較
assert!((umt_deg_to_rad(90.0) - std::f64::consts::PI / 2.0).abs() < EPSILON);
// 負の角度
assert!((umt_deg_to_rad(-90.0) - (-std::f64::consts::PI / 2.0)).abs() < EPSILON);
// 特殊なケース
assert!(umt_deg_to_rad(f64::NAN).is_nan());
}| #[test] | ||
| fn test_ncr_negative_r() { | ||
| umt_ncr(5, -3); | ||
| } |
There was a problem hiding this comment.
負のrに対するテストケースが不完全です
関数を呼び出すだけで、期待される動作の検証が行われていません。以下のいずれかの改善が必要です：
- パニックを期待する場合は
#[should_panic]属性を追加 - 特定の戻り値を期待する場合は
assert_eq!を追加
#[test]
+#[should_panic(expected = "negative value not allowed")]
fn test_ncr_negative_r() {
umt_ncr(5, -3);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| fn test_ncr_negative_r() { | |
| umt_ncr(5, -3); | |
| } | |
| #[test] | |
| #[should_panic(expected = "negative value not allowed")] | |
| fn test_ncr_negative_r() { | |
| umt_ncr(5, -3); | |
| } |
| #[test] | ||
| fn test_ncr_negative_n() { | ||
| umt_ncr(-5, 3); | ||
| } |
There was a problem hiding this comment.
負のnに対するテストケースが不完全です
関数を呼び出すだけで、期待される動作の検証が行われていません。以下のいずれかの改善が必要です：
- パニックを期待する場合は
#[should_panic]属性を追加 - 特定の戻り値を期待する場合は
assert_eq!を追加
#[test]
+#[should_panic(expected = "negative value not allowed")]
fn test_ncr_negative_n() {
umt_ncr(-5, 3);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| fn test_ncr_negative_n() { | |
| umt_ncr(-5, 3); | |
| } | |
| #[test] | |
| #[should_panic(expected = "negative value not allowed")] | |
| fn test_ncr_negative_n() { | |
| umt_ncr(-5, 3); | |
| } |
PR Type
Enhancement, Tests, Documentation
Description
umt_rustwith a collection of utility functions.math) and array utilities (array).umt_gcd,umt_lcm,umt_factorial, andumt_average.Cargo.tomlwith package metadata and set Rust edition to 2024.Changes walkthrough 📝
10 files
mod.rs
Introduced `range` module for array utilitiespackage/umt_rust/src/array/mod.rs
rangeand re-exported its contents.range.rs
Added `umt_range` function for integer rangespackage/umt_rust/src/array/range.rs
umt_rangefunction to generate a range of integers.lib.rs
Added `array` and `math` modules to librarypackage/umt_rust/src/lib.rs
arrayandmathmodules to the library.average.rs
Added `umt_average` function for averagespackage/umt_rust/src/math/average.rs
umt_averagefunction to calculate the average of a vectorof floats.
deg_to_rad.rs
Added `umt_deg_to_rad` function for degree-radian conversionpackage/umt_rust/src/math/deg_to_rad.rs
umt_deg_to_radfunction to convert degrees to radians.deviation_value.rs
Added `umt_deviation_value` function for deviation calculationspackage/umt_rust/src/math/deviation_value.rs
umt_deviation_valuefunction to calculate deviationvalues.
factorial.rs
Added `umt_factorial` function for factorial calculationspackage/umt_rust/src/math/factorial.rs
umt_factorialfunction to calculate factorials.factorize.rs
Added `umt_factorize` function for integer factorizationpackage/umt_rust/src/math/factorize.rs
umt_factorizefunction to factorize integers.gcd.rs
Added `umt_gcd` function for GCD calculationspackage/umt_rust/src/math/gcd.rs
umt_gcdfunction to calculate the greatest common divisor.lcm.rs
Added `umt_lcm` function for LCM calculationspackage/umt_rust/src/math/lcm.rs
umt_lcmfunction to calculate the least common multiple.2 files
test_average.rs
Added tests for `umt_average` functionpackage/umt_rust/tests/math/test_average.rs
umt_averagefunction.test_deg_to_rad.rs
Added tests for `umt_deg_to_rad` functionpackage/umt_rust/tests/math/test_deg_to_rad.rs
umt_deg_to_radfunction.3 files
Cargo.toml
Updated Cargo.toml with package metadata and editionpackage/umt_rust/Cargo.toml
and repository.
LICENSE
Added MIT license filepackage/umt_rust/LICENSE
index.md
Added project documentation indexpackage/umt_rust/doc/index.md