-
Notifications
You must be signed in to change notification settings - Fork 13
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.
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
Should we require the status list entries to be a multiple of the statuses per byte? #235
Comments
Mhh, I would not prefer to limit the user that he needs to think about what should be n so Here we are calculating the size based on the byteArray, there is no hardcoded reference how many elements should be managed. And since we have to set a value for the bit in the byte, it is 0. |
As having implemented this functionality in Credo i spent quite a chunk of time debugging why my status list was longer than I configured it. I thought it was a bug in the library, and finally i found that it goes per byte. So passing 4 is the same as 8, i think it's useful to throw an error and say that you should pass a size of multiple of 8 / bitSize. We can also implement this on the Credo layer, but i think unless there's a good reason why you would ever want to create a list of size 4 that ends up at 8 anyway, we should guide users to put in the correct length in the first place |
I would prefer a longer status list with more entries than intended, rather than forcing the use user to pick a specific value. Pick a random number has also no impact on the efficiency of the algorithm. When he says "I need a list of 799" elements, it will not break any use case when there are 800. From the functionality aspect, a longer list will break no test or system. Because you do not now the final length because of the compression after this. |
Okay would like to hear some input of others on this (@lukasjhan, @berendsliedrecht). If everyone/most of us think it's ok to have that misalignment between input and actual created list I will close this issue. |
If input a number that is not a multiple of 8, get a result that is different from the intended number of bits as @TimoGlastra mentioned. But I think it would be complicated to use if it throws an error. I think it would be better to handle it gracefully rather than throwing an error. |
e.g. if you have a status list with 4 status, and bit size 1, you encode this and then decode it, the decoded status list will have 8 entries, as you encode the status list per byte.
So I think we should maybe do a check that the
<totalStatus> % (8 / <bitsPerStatus>)
equals 0E.g. a status list of length 256, with 1 bit per status =
256 % (8 / 1)
= 0However a status list of length 4 with 1 bit per status =
4 % (8 / 1)
= 4This will ensure the initial status list will always equal the decoded status list
The text was updated successfully, but these errors were encountered: