Add blit_to() to @ArrayView

[Asterless]

Summary

• Fixes [Feature Request] add ArrayView::bilt_to #1958

New Method Addition:

• array/view.mbt: Implemented the blit_to method with logic for iterating over the source ArrayView and copying its elements to the destination array.

Testing Enhancements:

• array/view_test.mbt: Added multiple test cases for the blit_to method, covering scenarios such as basic usage, copying at the start or end of the destination array, handling empty source arrays, and full overwrites of the destination array.

[peter-jerry-ye-code-review]

Complex nested conditional logic makes the copy length calculation error-prone and hard to verify

Category
Correctness
Code Snippet
let copy_len = if src_len < dst_len {
if src_len < len {
src_len
} else {
len
}
} else if dst_len < len {
dst_len
} else {
len
}
Recommendation
Use a simpler approach with built-in min function: let copy_len = min(len, min(src_len, dst_len))
Reasoning
The current nested conditionals are difficult to reason about and verify correctness. Using min() makes the intent clear: copy at most len elements, but no more than what's available in source or fits in destination.

Missing bounds validation could lead to out-of-bounds access

Category
Correctness
Code Snippet
let src_len = self.length() - src_offset
let dst_len = dst.length() - dst_offset
Recommendation
Add validation: if src_offset < 0 || src_offset > self.length() { panic("src_offset out of bounds") } if dst_offset < 0 || dst_offset > dst.length() { panic("dst_offset out of bounds") }
Reasoning
If offsets are negative or exceed array bounds, the subtraction could result in negative lengths or incorrect calculations, leading to potential runtime errors or memory safety issues.

Inconsistent parameter naming and documentation mismatch

Category
Maintainability
Code Snippet
pub fn[T] ArrayView::blit_to(
self : ArrayView[T],
dst : ArrayView[T],
len~ : Int,
src_offset~ : Int = 0,
dst_offset~ : Int = 0
) -> Unit
Recommendation
Update documentation to match actual signature: /// Copies up to len elements from this ArrayView to a destination ArrayView starting at specified offsets. and consider renaming dst to dst_view for clarity
Reasoning
The documentation says 'destination array' but the parameter is actually an ArrayView. Clear parameter names and accurate documentation improve code maintainability and prevent confusion.

[coveralls]

Pull Request Test Coverage Report for Build 432

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 90.113%

---

Totals
Change from base Build 421:	0.0%
Covered Lines:	3518
Relevant Lines:	3904

---

💛 - Coveralls

[FlyCloudC]

The bilt_to function is expected to be more efficient than a for loop copy. However, the current implementation creates two new arrays and performs four copies, introducing additional space and time overhead. A more efficient approach might be to implement it as a direct bilt_to from self.buf to dst.buf.

[Asterless]

The bilt_to function is expected to be more efficient than a for loop copy. However, the current implementation creates two new arrays and performs four copies, introducing additional space and time overhead. A more efficient approach might be to implement it as a direct bilt_to from self.buf to dst.buf.

Ok, now I use UninitializedArray::unsafe_blit to achieve efficient blit_to method😊👍