DapperLib
[Question]: Unused value in the QueryAsync<T>
In the SqlMapper.Async Line 451 and
Line 1323 what is the purpose of calling Nullable.GetUnderlyingType, if it is not used afterwards?
And, if it's just a mistake, is it ok, if I open a PR for that? It looks like, it should be passed to the GetValue<T> method below it's call
It is a valid question. No idea, without digging. Possibly it should replace the effective-type a few lines lower, but: I'd need to investigate.
I've read through the code and seems like it just could be safely removed. The GetValue<T> already contains the same code to check the underlying type of the nullable to decide which default value to return.
The only theoretical difference in the observable behavior could have been the case, when the passed effectiveType is a null, which will throw an exception before the first call to ReadAsync. And if it's removed, the same exception would be thrown in the GetValue<T> method
@mgravell I took a closer look at this and confirmed that GetValue<T> already handles nullable types internally.
Specifically:
- It explicitly allows nullable value types when
val is null
(Line1378) - It unwraps nullable types again when calling
Convert.ChangeType
(Line1387)
Because of that, the call to
Nullable.GetUnderlyingType(effectiveType) ?? effectiveType
inside QueryAsync<T> appears to be redundant, since the result is not used and does not affect behavior or exception timing.
Happy to submit a small PR to remove this line if that aligns with the intended direction.
