Skip to content

test: add multiply and divide functions - #38

Open
CristianAmbrosini wants to merge 1 commit into
mainfrom
test/telemetry-check
Open

test: add multiply and divide functions#38
CristianAmbrosini wants to merge 1 commit into
mainfrom
test/telemetry-check

Conversation

@CristianAmbrosini

Copy link
Copy Markdown
Owner

Summary

  • Add multiply and divide functions to calculator module
  • Testing telemetry event publishing

🤖 Generated with Claude Code

@sonar-review-dev18

sonar-review-dev18 Bot commented Apr 27, 2026

Copy link
Copy Markdown

Summary

Adds two basic calculator functions to the module: multiply() and divide(). The divide function includes guard logic to raise a ValueError on division by zero. No other changes to existing code.

Note: The PR description mentions "testing telemetry event publishing," but the actual diff shows only the two new functions with no telemetry instrumentation. Reviewers may want to clarify whether telemetry logging is expected here or handled elsewhere.

What reviewers should know

Quick scan:

  • All changes are in calculator.py (lines 41–50)
  • Two straightforward functions with no complex logic or dependencies
  • The divide function is the only one with error handling

Minor style note:
The new functions lack type hints, while the existing process_scores() function uses them (scores: list[int], return type). Consider whether to add hints for consistency.

For telemetry reviewers:
The description mentions "testing telemetry event publishing," but no telemetry calls are present in the diff. Verify whether this is intentional, tested separately, or if instrumentation is expected here.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@sonarqube-cloud-dev18

Copy link
Copy Markdown

@sonar-review-dev18 sonar-review-dev18 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Real issues here that need fixing before merge. The new functions shadow existing definitions — Python takes the last binding, so the original typed versions at lines 9 and 29 are now unreachable dead code.

🗣️ Give feedback

Comment thread calculator.py
Comment on lines +43 to +44
def multiply(a, b):
return a * b

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Logic duplication: multiply is already defined at line 29 with type annotations (a: int, b: int) -> int). In Python the last definition wins, so the typed version is now dead code — callers silently get this untyped one. Remove this definition (the one at line 29 is the right one to keep).

  • Mark as noise

Comment thread calculator.py
Comment on lines +47 to +50
def divide(a, b):
if b == 0:
raise ValueError("Cannot divide by zero")
return a / b

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Logic duplication: divide is already defined at line 9 with type annotations (a: float, b: float) -> float) and identical zero-division logic. This definition shadows it, making the typed version dead code. Remove this duplicate.

Missing test coverage: Whichever divide survives has no tests for either the happy path or the zero-division guard. A regression (e.g. the guard being dropped) would go undetected. Add at least: assert divide(6, 2) == 3 and a check that divide(1, 0) raises ValueError.

  • Mark as noise

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.

1 participant