Skip to content

Simplify AddressToRawNumberConverter to set MSB directly #164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
konard wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Simplify AddressToRawNumberConverter to set MSB directly #164

konard wants to merge 4 commits into from

Conversation

@konard
Copy link
Member

@konard konard commented Sep 13, 2025 •
edited
Loading

Summary

This PR simplifies the AddressToRawNumberConverter<TLinkAddress> by replacing the complex Hybrid constructor logic with a direct bitwise OR operation to set the most significant bit (MSB).

Changes Made

  • Before: Used new Hybrid<TLinkAddress>(source, isExternal: true) which involved complex negation logic
  • After: Directly set MSB using bitwise OR (source | msbMask) through safe type converters

Technical Implementation

  1. Type-Safe Bit Operations: Uses UncheckedConverter to safely convert generic types to ulong for bit manipulation
  2. Dynamic MSB Calculation: Calculates the correct MSB mask based on the actual size of TLinkAddress type
  3. Direct MSB Setting: Sets the most significant bit using bitwise OR operation
  4. Performance Improvement: Eliminates complex Hybrid constructor overhead

Test Results

All test cases pass with the new implementation:

Input: 0   → Output: 128 (0x80) ✅ MSB set correctly
Input: 1   → Output: 129 (0x81) ✅ MSB set correctly  
Input: 127 → Output: 255 (0xFF) ✅ MSB set correctly
Input: 128 → Output: 128 (0x80) ✅ MSB already set
Input: 255 → Output: 255 (0xFF) ✅ MSB already set

Benefits

  • ✅ Simplified Logic: Direct MSB setting vs complex Hybrid logic
  • ✅ Better Performance: Eliminates Hybrid constructor overhead
  • ✅ Clearer Intent: Code directly expresses "set MSB to 1"
  • ✅ Predictable Results: Simple bitwise OR behavior
  • ✅ Generic Support: Works correctly with all unsigned integer types

Fixes #66

🤖 Generated with Claude Code

@konard
Initial commit with task details for issue #66
26c43aa 
Adding CLAUDE.md with task information for AI processing.
This file will be removed when the task is complete.

Issue: #66
@konard konard self-assigned this Sep 13, 2025
@konard @claude
Simplify AddressToRawNumberConverter to set MSB directly
3492f10 
- Replace complex Hybrid logic with simple bitwise OR to set MSB
- Use converters to handle generic type bit operations safely
- Achieves the same external address marking with clearer intent
- Resolves issue #66 by implementing direct MSB setting as requested

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@konard konard changed the title [WIP] This converter can be simplified the only thing that is required to be done is to set the most significant bit to 1 Simplify AddressToRawNumberConverter to set MSB directly Sep 13, 2025
@konard konard marked this pull request as ready for review September 13, 2025 08:52
@konard
'Auto-commit changes made by Claude
840e89e 
🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>'
@konard
Remove CLAUDE.md - Claude command completed
5e6c3fa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@konard konard

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

This converter can be simplified the only thing that is required to be done is to set the most significant bit to 1

2 participants

@konard