Skip to content

[AI Task] [Tizen.Pims.Calendar] Remove Convert.ChangeType boxing from CalendarRecord Get/Set - #7800

Open
JoonghyunCho wants to merge 1 commit into
mainfrom
ai-task/issue-7710
Open

[AI Task] [Tizen.Pims.Calendar] Remove Convert.ChangeType boxing from CalendarRecord Get/Set#7800
JoonghyunCho wants to merge 1 commit into
mainfrom
ai-task/issue-7710

Conversation

@JoonghyunCho

Copy link
Copy Markdown
Member

Summary

Removes the Convert.ChangeType / Convert.To* conversion paths from CalendarRecord.Get<T>(uint) and Set<T>(uint, T). Each branch already verifies the exact type via typeof(T) == typeof(X), so the value is now converted with a direct cast ((T)(object)val) instead of going through the culture-aware IConvertible dispatch. Get<T> also drops the intermediate object parsedValue local and returns directly from each branch.

Per-call effect (e.g. Get<int>): no Convert.ChangeType call, no IConvertible vtable dispatch, no CultureInfo lookup. For Get<string>/Set<string> and the CalendarTime branches the redundant conversion round-trip disappears entirely (reference-type cast). Get/Set run once per record field, so iterating a CalendarList multiplies the savings by (records x fields).

This applies the identical pattern already approved for the sibling module in #7762 (ContactsRecord, issue #7667).

Changes

  • src/Tizen.Pims.Calendar/Tizen.Pims.Calendar/CalendarRecord.cs
    • Get<T>: each type branch now ends with return (T)(object)val;; removed the object parsedValue local, the Convert.ChangeType calls (5), and the trailing unbox return (T)parsedValue;. The unsupported-type branch still throws NotSupported as before.
    • Set<T>: Convert.ToString/ToInt32/ToInt64/ToDouble replaced with direct casts; Convert.ChangeType in the CalendarTime branch replaced with (CalendarTime)(object)value. The string branch uses value as string ?? string.Empty to preserve the exact prior behavior of Convert.ToString(object), which maps null to string.Empty (so native SetString still receives an empty string, not null).

Mode

Refactoring

Verification

  • Build: passed (0 errors, 0 warnings)
  • Tests: N/A
  • Benchmark: skipped (sdb error: no device connected)

Public API signatures (T Get<T>(uint), void Set<T>(uint, T)) are unchanged. Every direct cast is guarded by the exact typeof(T) check, so no InvalidCastException is reachable on paths that previously succeeded; normal-path results are bit-identical, including Get<string> returning null when the native value is NULL and Convert.ChangeType's same-type shortcut for CalendarTime (not IConvertible).

Fixes #7710

🤖 Generated with Claude Code

Fixes #7710)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JoonghyunCho

Copy link
Copy Markdown
Member Author

⚠️ Benchmark skipped: Encountered an issue with sdb device connection/deployment/execution, so the benchmark could not be run. Manual benchmark verification is required. Error: sdb devices reports no attached devices (empty device list), so the comparative benchmark could not be deployed or executed.

@JoonghyunCho

Copy link
Copy Markdown
Member Author

🤖 [AI Review]

Reviewed — no findings.

Scope checked:

  • All 10 direct casts (5 Get<T> branches, 5 Set<T> branches) are dominated by an exact typeof(T) == typeof(X) guard, so no path that previously succeeded can now throw InvalidCastException.
  • value as string ?? string.Empty in the Set<T> string branch preserves the Convert.ToString(object) null-to-empty mapping, so native SetString still never receives null.
  • Get<string> still surfaces a native NULL as a null return — (T)(object)val matches the prior Convert.ChangeType null behavior.
  • After removing the parsedValue local, every Get<T> branch returns or throws (the unsupported-type branch still throws NotSupported), so control flow remains complete.
  • The CalendarTime branches previously relied on the Convert.ChangeType same-type shortcut (the type is not IConvertible); the direct cast is behavior-identical, including null pass-through.
  • Public API signatures T Get<T>(uint) and void Set<T>(uint, T) are unchanged.

No 🔴 critical issues, no 🟡 suggestions to flag.


Automated review — final merge decision rests with human reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

1 participant