search

iddm / thread-priority

A simple Cross-platform thread schedule and priority library for rust.
MIT License
110 stars 20 forks source link

ThreadPriorityValue::MIN/ThreadPriorityValue::MAX have unexpected types #38

Open nazar-pc opened 8 months ago

nazar-pc commented 8 months ago

I believe those should have ThreadPriorityValue type, not u8. It is always possible to convert it to u8 later if desired, but converting u8 to ThreadPriorityValue requires unwrap() or expect(), which is inconvenient and unnecessary.

P.S. Clippy warning here has nothing to do with From<u8>, it suggests you to do impl From<ThreadPriorityValue> for u8: https://github.com/iddm/thread-priority/blob/74cf570b36bb12d0dc3ea2d406d21569516aff96/src/lib.rs#L264-L271

iddm commented 8 months ago

Thank you for reporting this, I'll have to check. It has been a long time since I last worked on this project. Not to mention that I privately am rewriting it to something much simpler than it is now.

iddm commented 7 months ago

So I checked the code, and I remember the reason why I did it this way.

So the ThreadPriorityValue was supposed to be a universal numeric value in something like percentage, varying from 0 to 99 inclusively. The higher the value - the higher the priority should be, and vice versa. The reason I implemented only the Into<u8> and not From<ThreadPriorityValue> is because not all the values of u8 are correct for ThreadPriorityValue, but only in the range of 0..=99, so any value from within the range of 100.==255 will be incorrect. Hence, there was no safe and guaranteed conversion from ThreadPriorityValue to u8; the opposite was true from u8 to ThreadPriorityValue. And this is why there is TryFrom.

I understand that the unwrap() or expect() isn't something one might want to have all the time, but unfortunately, there is no safe way to convert from one to another. We could have some clamping the values over 99, but this might have implicit meaning for some people, who might think that there is indeed a difference between 99 and 100 and any other value higher than 99.

nazar-pc commented 7 months ago

The reason I implemented only the Into<u8> and not From<ThreadPriorityValue> is because not all the values of u8 are correct for ThreadPriorityValue

What you are talking about is impl From<u8> for ThreadPriorityValue, which if added would be a problem indeed.

What clippy is talking about is impl From<ThreadPriorityValue> for u8, which doesn't violate any expectations and does exactly the same thing as impl Into<u8> for ThreadPriorityValue in the end, just more awkward sometimes.