Basic reflect types for package consumers to use #19
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19 +/- ##
==========================================
+ Coverage 29.94% 40.14% +10.2%
==========================================
Files 4 4
Lines 521 543 +22
==========================================
+ Hits 156 218 +62
+ Misses 346 300 -46
- Partials 19 25 +6
Continue to review full report at Codecov.
|
|
I think that's a good approach but it also comes with a lot of challenges. The main one being that any function receiving a buffer might have to do the check and convert the date, so yo might switch back and forth many times within the processing of a file. The other issue is the allocation of lots of temporary buffers (but we can work around that issue by seeing a scratch buffer). I took a similar approach in my initial version: https://github.com/mattetti/audio/blob/master/pcm_buffer.go The one thing I am not a fan of is to bring an entire package just to get a type lookup table. I think we'd be better off using an enum of constants here. |
|
I have to review what I did, but I'm thinking I might be able to implement
or use our own iota map (don't have code in front of me right now). I'll
check later and see, I used reflect only because it's stdlib and quick but
it may make sense to wrap the types anyways
…On Sat, May 4, 2019, 6:46 AM Matt Aimonetti ***@***.***> wrote:
I think that's a good approach but it also comes with a lot of challenges.
The main one being that any function receiving a buffer might have to do
the check and convert the date, so yo might switch back and forth many
times within the processing of a file. The other issue is the allocation of
lots of temporary buffers (but we can work around that issue by seeing a
scratch buffer). I took a similar approach in my initial version:
https://github.com/mattetti/audio/blob/master/pcm_buffer.go
The one thing I am not a fan of is to bring an entire package just to get
a type lookup table. I think we'd be better off using an enum of constants
here.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#19 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAMLNBZYGYRFCQZYAI5NVTLPTWHUZANCNFSM4HKYHBCQ>
.
|
|
DeepEqual on two slices is an O (n) operation so it may bring a strong impact to the performance. |
|
Deep equal is only used in tests as you can see it was already being used
by the maintainers. Otherwise only the reflect kind structure was being
used and I'm going to replace it with an enum
…On Sat, May 4, 2019, 6:10 PM 方泓睿 ***@***.***> wrote:
DeepEqual on two slices is an O (n) operation so it may bring a strong
impact to the performance.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#19 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAMLNB4UB5VILI23UZAYKZ3PTYXZLANCNFSM4HKYHBCQ>
.
|
|
Oh sorry I didn't see that it's a test. |
This feature derived from conversation in go-audio/wav#20
I am using reflective types but performing no reflection, there should be no performance difference or compatibility issues, outside of projects conforming to the interface.
The intention for these changes would be for the Write() encoder function to be able to ask the type so it can utilize
AsIntBufferonly when required - allowing Write() to accept the buf Interface instead of type so it can accept any type but then silently convert to int before passing it on.This is meant to be an early step to support any type in write without actually writing any type.