Skip to content

Conversation

@lichaofan2008
Copy link

The calculation of 10% should not rely on the preceding numerical values. User expectation: 10% + 10% = 0.2
10%的计算不应该依赖其前面的数值,用户期望:10%+10%=0.2。

Bug: https://pms.uniontech.com/bug-view-347383.html

The calculation of 10% should not rely on the preceding numerical values. User expectation: 10% + 10% = 0.2
10%的计算不应该依赖其前面的数值,用户期望:10%+10%=0.2。

Bug: https://pms.uniontech.com/bug-view-347383.html
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @lichaofan2008, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码修改涉及计算器中百分比运算的逻辑变更。以下是对该代码变更的详细审查和改进建议:

1. 语法逻辑审查

  • 变更内容:代码将 val2 * (val1 * HNumber("0.01")) 修改为 (val1 * HNumber("0.01")),并注释掉了 val2 *
  • 逻辑分析
    • 旧逻辑val2 * (val1 * 0.01)。这通常用于计算类似 "50 + 10%" 的场景,即 50 + (50 * 0.10),结果为 55。这是一种"累加百分比"的逻辑。
    • 新逻辑val1 * 0.01。这直接计算输入数值的百分比。例如输入 "10%",结果为 0.1。
  • 注释意图:注释提到 // 10%的计算不应该依赖其之前的数值,用户期望:10%+10%=0.2。这表明开发者希望将百分比运算符从"基于当前栈顶值的运算"(即链式运算)修改为"独立的数值转换"。
  • 潜在问题:虽然修改符合注释中 10%+10%=0.2 的期望(即 0.1 + 0.1 = 0.2),但它破坏了标准计算器的行为。大多数标准计算器(如Windows计算器)在输入 100 + 10% 时,期望结果是 110,而不是 100.1。如果采用新逻辑,100 + 10% 将变成 100 + 0.1 = 100.1,这可能不符合大多数用户在科学或标准计算模式下的直觉。

2. 代码质量审查

  • 注释风格:代码中使用了 /* ... */ 来注释掉代码,这通常用于临时的调试。如果该修改是永久性的,建议直接删除被注释的代码 val2 *,保持代码整洁。
  • 硬编码HNumber("0.01") 是硬编码的字符串字面量。虽然在此处性能影响微乎其微,但定义一个常量或静态变量会更具可读性和维护性。

3. 代码性能审查

  • 对象构造HNumber("0.01") 在每次调用时都会构造一个新的对象。如果此函数在大量计算循环中被高频调用,频繁的字符串解析和对象构造会带来微小的性能开销。
  • 建议:将 0.01 定义为静态常量,例如 static const HNumber PERCENT_FACTOR("0.01");,或者查看 HNumber 是否支持直接从浮点数构造。

4. 代码安全审查

  • 数值溢出/精度:使用了 HNumber(通常指高精度数学库),这本身比原生 double 更安全。但需确认 val1 * HNumber("0.01") 是否会产生意外的精度截断或溢出,特别是在处理极大或极小数值时。
  • 异常处理checkOperatorResult 函数被调用来包裹计算结果。这表明库内部可能有对除零、溢出等错误的处理机制。修改逻辑后,需确认 val1 单独参与运算时,所有边界情况(如 val1 为无穷大或非数字)依然能被 checkOperatorResult 正确捕获。

5. 综合改进建议

修改后的代码建议:

// 建议在文件头部或类中定义常量
// static const HNumber PERCENT_FACTOR("0.01"); 

// ... 在函数内部 ...
Quantity Evaluator::exec(const QVector<Opcode> &opcodes, ...) {
    // ...
    case Opcode::Percent:
        // ... (获取 val1 的代码) ...
        
        // 改进点:
        // 1. 移除被注释的旧代码,保持整洁。
        // 2. 使用常量替代字符串字面量(如果已定义)。
        // 3. 明确逻辑:这里执行的是纯粹的百分比转换,不涉及累加。
        
        // 逻辑说明:将输入值转换为纯小数形式 (例如 10 -> 0.1)
        val1 = checkOperatorResult(val1 * HNumber("0.01")); 
        
        // 警告:此逻辑修改意味着 "100 + 10%" 将变为 100.1 而非 110。
        // 如果这是为了修复特定Bug或符合特定需求,请确保已更新相关的单元测试。
        
        m_standardPercent = val1;
        stack.push(val1);
        break;
    // ...
}

总结
这个修改将百分比运算符从"上下文相关的运算符"(依赖栈中前一个数值 val2)变成了"一元运算符"(仅依赖当前输入 val1)。这改变了计算器的核心行为模式。

  • 如果项目目标是实现类似编程语言中 % 操作符的行为(即取模或单纯的数值转换),那么这个修改是正确的。
  • 如果项目目标是模拟标准手持计算器,这个修改可能会导致用户困惑,因为 100 + 10% 的结果将不再符合常规直觉(期望110,得到100.1)。

建议在合并此代码前,务必更新相关的单元测试用例,特别是针对 A + B% 这种场景的测试,确保行为变更符合产品需求。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lichaofan2008, max-lvs

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@lichaofan2008
Copy link
Author

/merge

@deepin-bot deepin-bot bot merged commit 31fab1d into linuxdeepin:release/eagle Jan 20, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants