asmirnov82 opened a new issue, #35540:
URL: https://github.com/apache/arrow/issues/35540

   ### Describe the enhancement requested
   
   After taking a closer look at BitUtility class I have some concern that 
BitUtility should be a public API. 
   
   And here are reasons:
   
   1) BitUtility is implemented with an assumption of being used with Bitmap 
only, when we know that the number of meaningful bits in a provided Span is 
always less than Max.Int (because each bit corresponds to the validity of value 
in Value.Span, which length is limited by Int). 
   All methods in BitUtility uses int as input for bit index and also int as 
lenght. For example:
   
   `int CountBits(ReadOnlySpan<byte> data, int offset, int length)
   void SetBit(Span<byte> data, int index)
   bool GetBit(ReadOnlySpan<byte> data, int index)`
   
   and so on.
   
   In general case the amount of bit inside a Span can be Max.Int * 8 and 
correct methods for working with bits inside a span should have long value for 
bit index and for length, like:
   
   `long CountBits(ReadOnlySpan<byte> data, long offset, long length)
   void SetBit(Span<byte> data, long index)
   bool GetBit(ReadOnlySpan<byte> data, long index)`
   
   2) There aren't any argument checking in all other methods (however, there 
are debug.asserts - I think it's done due to perfomance reason)
   
   So the proposal is to make  the whole BitUtility class internal, this will 
also allow to refactor this code in any way we want.
   
   
   ### Component(s)
   
   C#


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to