BitList structure #143
BitList structure #143
Conversation
|
Thank you for the bit list, I'll take a look at it when I have more time, or maybe someone else will review it in the meantime! |
Equality operator stack overflow fixed
BitList.cs tests
Changes (100% safe code):
- 1 new ctor: BitList.BitList(ValueType unmanagedNonPrimitiveBits)
- 2 new methods:
- T[] ToUnmanagedNonPrimitiveValueArray<T>() where T : unmanaged
- void CopyTo<T>(T[] unmanagedNonPrimitiveValueArray, int arrayIndex) where T : unmanaged
New ctor:
The new ctor create a BitList instance by converting the ValueType param (must be unmanaged and non primitive) in a safe buffer.
New method 1:
Convert the BitList content to an array of a specified unmanaged and non primitive value type (no pointers).
New method 2:
Copy the BitList content to an array of a specified unmanaged and non primitive value type (no pointers);
|
Issues
======
- Added 19
Complexity increasing per file
==============================
- DataStructures.Tests/BitListTest.cs 3
- DataStructures/BitList.cs 16
See the complete overview on Codacy |
| } | ||
| else | ||
| { | ||
| try |
There was a problem hiding this comment.
Codacy found an issue: Refactor this code to not nest more than 3 control flow statements.
| /// <summary> | ||
| /// Ctor (<paramref name="unmanagedNonPrimitiveBits"/> is an unmanaged value type object). | ||
| /// </summary> | ||
| public BitList(ValueType unmanagedNonPrimitiveBits) |
There was a problem hiding this comment.
Codacy found an issue: The Cyclomatic Complexity of this constructor is 12 which is greater than 10 authorized.
| var text = string.Empty; | ||
| foreach (var bit in ToBooleanArray()) | ||
| { | ||
| text += bit ? "1" : "0"; |
There was a problem hiding this comment.
Codacy found an issue: Use a StringBuilder instead.
| /// </summary> | ||
| /// <typeparam name="T"></typeparam> | ||
| /// <returns></returns> | ||
| public T[] ToUnmanagedNonPrimitiveValueArray<T>() where T : unmanaged |
There was a problem hiding this comment.
Codacy found an issue: Refactor this method to reduce its Cognitive Complexity from 19 to the 15 allowed.
| var result = new List<T>(); //A list of unmanaged non primitive value type objects. | ||
| for (var i = 0; i < bytes.Count; i += size) | ||
| { | ||
| if (typeof(T).IsEnum) |
There was a problem hiding this comment.
Codacy found an issue: Refactor this code to not nest more than 3 control flow statements.
| } | ||
| else | ||
| { | ||
| throw new IndexOutOfRangeException(); |
There was a problem hiding this comment.
Codacy found an issue: 'System.IndexOutOfRangeException' should not be thrown by user code.
| break; | ||
|
|
||
| default: | ||
| case "Int32": |
There was a problem hiding this comment.
Codacy found an issue: Remove this empty 'case' clause.
| get => Get(index, count); | ||
| set | ||
| { | ||
| if (count >= value.Count) |
There was a problem hiding this comment.
Codacy found an issue: Use the '?:' operator here.
| } | ||
| else | ||
| { | ||
| throw new ArgumentOutOfRangeException(); |
There was a problem hiding this comment.
Codacy found an issue: Use a constructor overloads that allows a more meaningful exception message to be provided.
| var bits = new BitList(1F); | ||
| var array = new float[1]; | ||
| bits.CopyTo(array, 0); | ||
| Assert.IsTrue(array.Length == 1 && array[0] == 1); |
There was a problem hiding this comment.
Codacy found an issue: Do not check floating point equality with exact values, use a range instead.
| /// </summary> | ||
| public BitList Get(int index, int count) | ||
| { | ||
| if (!IsEmpty && index < bits.Length && index >= 0 && index + count <= bits.Length - index && count > 0) |
There was a problem hiding this comment.
Codacy found an issue: Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
| /// </summary> | ||
| /// <typeparam name="T"></typeparam> | ||
| /// <returns></returns> | ||
| public T[] ToUnmanagedNonPrimitiveValueArray<T>() where T : unmanaged |
There was a problem hiding this comment.
Codacy found an issue: The Cyclomatic Complexity of this method is 16 which is greater than 10 authorized.
| return ToBooleanArray().ToList().GetEnumerator(); | ||
| } | ||
|
|
||
| private class Unmanaged<T> where T : unmanaged { } |
There was a problem hiding this comment.
Codacy found an issue: 'T' is not used in the class.
| /// </summary> | ||
| public void Set(int index, BitList value) | ||
| { | ||
| if (!IsEmpty && index < bits.Length && index >= 0 && index + value.Count <= bits.Length - index && value.Count > 0) |
There was a problem hiding this comment.
Codacy found an issue: Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
| @@ -0,0 +1,1750 @@ | |||
| using System; | |||
There was a problem hiding this comment.
| break; | ||
|
|
||
| default: | ||
| case "UInt32": |
There was a problem hiding this comment.
Codacy found an issue: Remove this empty 'case' clause.
|
Thanks for the BitList! I see that it's a wrapper over BitArray, why would anyone want one? This repository focuses on implementing data structures, but this implementation uses BitArray under the hood. Also, there are methods that are not essential for Bit(Array|List) and I would suggest removing them (file IO, copying, constructor overloads). |
|
Hi, if BitArray implementation is a problem I can base the structure on a
list or an array.
Il lun 29 giu 2020, 05:53 Andrii Siriak <notifications@github.com> ha
scritto:
… Thanks for the BitList! I see that it's a wrapper over BitArray, why would
anyone want one? This repository focuses on implementing data structures,
but this implementation uses BitArray under the hood. Also, there are
methods that are not essential for Bit(Array|List) and I would suggest
removing them (file IO, copying, constructor overloads).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#143 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMNWXI73UCMT5TLQMMG2HSDRZAF35ANCNFSM4NGFJERA>
.
|
|
I can also remove IO methods and reduce constructors to half using
BitList(params primitivetype[]) instead of BitList(primitivetype) and
BitList(primitivetype[]), or i can just leave the BitList(ValueType)
constructors and edit it to accept primitive types values but this can decrease
perfomances.
Il lun 29 giu 2020, 10:08 Lorenzo Lotti <lorenzocinghiale04@gmail.com> ha
scritto:
… Hi, if BitArray implementation is a problem I can base the structure on a
list or an array.
Il lun 29 giu 2020, 05:53 Andrii Siriak ***@***.***> ha
scritto:
> Thanks for the BitList! I see that it's a wrapper over BitArray, why
> would anyone want one? This repository focuses on implementing data
> structures, but this implementation uses BitArray under the hood. Also,
> there are methods that are not essential for Bit(Array|List) and I would
> suggest removing them (file IO, copying, constructor overloads).
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#143 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AMNWXI73UCMT5TLQMMG2HSDRZAF35ANCNFSM4NGFJERA>
> .
>
|
|
The BitList data structure, besides being a list of bits, is mainly used
for converting any unmanaged type to any other unmanaged type (including
any unmanaged structure in or out the .NET libraries)
Il lun 29 giu 2020, 10:24 Lorenzo Lotti <lorenzocinghiale04@gmail.com> ha
scritto:
… I can also remove IO methods and reduce constructors to half using
BitList(params primitivetype[]) instead of BitList(primitivetype) and
BitList(primitivetype[]), or i can just leave the BitList(ValueType)
constructors and edit it to accept primitive types values but this can decrease
perfomances.
Il lun 29 giu 2020, 10:08 Lorenzo Lotti ***@***.***> ha
scritto:
> Hi, if BitArray implementation is a problem I can base the structure on a
> list or an array.
>
> Il lun 29 giu 2020, 05:53 Andrii Siriak ***@***.***> ha
> scritto:
>
>> Thanks for the BitList! I see that it's a wrapper over BitArray, why
>> would anyone want one? This repository focuses on implementing data
>> structures, but this implementation uses BitArray under the hood. Also,
>> there are methods that are not essential for Bit(Array|List) and I would
>> suggest removing them (file IO, copying, constructor overloads).
>>
>> —
>> You are receiving this because you authored the thread.
>> Reply to this email directly, view it on GitHub
>> <#143 (comment)>,
>> or unsubscribe
>> <https://github.com/notifications/unsubscribe-auth/AMNWXI73UCMT5TLQMMG2HSDRZAF35ANCNFSM4NGFJERA>
>> .
>>
>
|
|
Based on the information you provided, I suggest:
|
|
Roger that
Il mar 30 giu 2020, 20:42 Andrii Siriak <notifications@github.com> ha
scritto:
… Based on the information you provided, I suggest:
- Base implementation on an array or something other than BitArray for
educational purposes
- Remove methods related to file IO because they violate SRP
- Reduce the number of constructors because this class is too cluttered
- Fix Codacy comments and build (I cannot merge before all checks pass)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#143 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMNWXI3SVM2RDAF5EGJXBJTRZIWXVANCNFSM4NGFJERA>
.
|
BitList data structure.