Closed sn4k3 closed 2 years ago
I see what you're saying about FieldOffset
, but why not start by simply making it configurable, rather than introduce an entirely new attribute that does the same thing?
I see what you're saying about
FieldOffset
, but why not start by simply making it configurable, rather than introduce an entirely new attribute that does the same thing?
Definitely FieldOffset
can be improved to have the same fields and produce the same result, however it default behavior must be untouch to not break current uses, which means by default Rewind
must be true
breaking the common logic of stream.seek
and file read/write on pure streams usages, that can lead user to error if they not read the documentation of the FieldOffset
.
Either way i'm happy with one, just tell if you want me to repatch to improve FieldOffset
instead and i will rewrite it :)
I guess my feeling is that introducing a new attribute is a lot just to change the default behavior. Maybe start by adding configuration and then at some point we can simply "alias" a new attribute if it makes sense.
Sure, i will close this PR and send a new one in about some mins with the improvements on FieldOffset
This pull request implements #178 which allows to jump to addresses and make use of
stream.Seek()
when required. Since i have files pointing to tables this will help having a master class rather that multiple classes and jump in between.Why not improve the FieldOffset instead?
I have tought about that, but that attribute name leads to confusion (at least for me), when I read FieldOffset i always think in offset the stream by a set amount from current position and keep going, instead it is setting the absolute position and reset it position to where it was before.
The
FieldPosition
follows and do exactly whatstream.Seek
do by default and so developer don't need to overthink.To replicate
FieldOffset
declare as[FieldPosition(offset, true)]
Arguments
Example:
Hope that makes sense :)