Skip to content
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

Improving constraint for BitHelper #20

Closed
NN--- opened this issue Sep 25, 2018 · 8 comments
Closed

Improving constraint for BitHelper #20

NN--- opened this issue Sep 25, 2018 · 8 comments
Assignees

Comments

@NN---
Copy link
Contributor

NN--- commented Sep 25, 2018

public static bool GetBit<T>(T bits, byte idx) where T : IConvertible

Just wondering, did you want to specify T : unmanaged perhaps ?
Check here:
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/generics/constraints-on-type-parameters
https://blogs.msdn.microsoft.com/seteplia/2018/06/12/dissecting-new-generics-constraints-in-c-7-3/

@dahall dahall self-assigned this Sep 25, 2018
@dahall
Copy link
Owner

dahall commented Sep 25, 2018

Thanks for bringing this to my attention. I hadn't seen this yet. I'll look to implement in the next release along with all the handle changes. If you have time to do a PR for this, I'd be very appreciative.

@NN---
Copy link
Contributor Author

NN--- commented Sep 25, 2018

Did you use IConvertible in other places where thr meaning was unmanaged constraint ?

@dahall
Copy link
Owner

dahall commented Sep 25, 2018

I tried to follow the pattern using struct, IConvertible constraints whenever specifying enum or number types. I used the struct constraint and exceptions when looking for blittable types.

@dahall
Copy link
Owner

dahall commented Sep 25, 2018

In this particular BitHelper extension, I was attempting to constrain to the set of items that could be converted to a number.

@NN---
Copy link
Contributor Author

NN--- commented Sep 26, 2018

So BitHelper definitely should use unmanaged to cover all blittable types.
As for enum there is Enum constraint in C# 7.3 and interesting workaround in older version.

https://stackoverflow.com/a/28527552

@NN---
Copy link
Contributor Author

NN--- commented Sep 26, 2018

Actually if I understand correctly BitHelper is intended for types smaller or equal than Int64 ?

Is it correct ?
If yes, then a correct answer is to just have overloads for all primitive types and not using IConvertible.
In case you say it is too much code to write, just use T4.
See how CodeJam uses it to generate all required overloads: https://github.com/rsdn/CodeJam/blob/master/Main/src/Algorithms/Algorithms.EqualRange.tt

@NN---
Copy link
Contributor Author

NN--- commented Sep 26, 2018

There is also possible NRE since you can pass null in 'bits' parameter.

@dahall
Copy link
Owner

dahall commented Nov 28, 2018

To simplify and help make more foolproof, I have extended the list of constraints to struct, IComparable, IComparable<T>, IConvertible, IEquatable<T>, IFormattable in 2.0. Not perfect, but better.

@dahall dahall closed this as completed Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants