add int/uint/float subtype export (int{8,16,32,64}, uint{8,16,32,64}, float{32,64}) and TypedObj #5
Conversation
There was a problem hiding this comment.
Hi @deepkolos,
Thanks for your contribution. I really like the idea of the proposed fixed-size types.
I added some comments on your code.
| @@ -331,6 +346,31 @@ export function TypedMap<V>(keyT: Type<number|string>, valueT: Type<V>): Collect | |||
| }; | |||
| } | |||
|
|
|||
| export function TypedObj<T>(objT: Obj<Type<any>>): Collection<T> { | |||
There was a problem hiding this comment.
I think this should return a Type<Obj<any>> as Struct does. Therefore the type T can also be removed.
There was a problem hiding this comment.
yes, before the commit fix: TypedObj's type error, but i use that in application code, typescript throw a type error
so i use in this way, to fix the error
There was a problem hiding this comment.
maybe this is not the right way to fix that problem
There was a problem hiding this comment.
Can you please provide a minimal example where this error occurs? I'm not quite sure, if I understand your issue here.
src/types.ts
Outdated
| enc(buf: WriteBuffer, v: number): void { | ||
| buf.putUi8(tag); | ||
|
|
||
| switch(tag) { |
There was a problem hiding this comment.
I don't like that the switch is called for every enc call although the tag is fixed for the returned object. Maybe it is better to implement the fixed-size type (Int8, Int16, ...) and use them in Int.
You also miss the opportunity to encode a small integer with a "fixnum" tag (see msgpack spec).
There was a problem hiding this comment.
thanks for pointing out, i adjust that later
src/types.ts
Outdated
| }, | ||
|
|
||
| dec(buf: ReadBuffer): number { | ||
| return type.dec(buf); |
There was a problem hiding this comment.
Can you please provide some more details how the suggested fixed-size types should behave?
For example, the Int8 type uses the same decoding function as the Int type. This can result in an value that does not fit in 8 bits. What is the advantage of the Int8 type if you can't be sure you always get an 8-bit integer?
Maybe you can elaborate a little bit more on this.
There was a problem hiding this comment.
my situation is the 90Hz time value is Uint32, even overflowed is ok, but the length need to be Uint32, even value is small
There was a problem hiding this comment.
For your situation the overflow might be ok. But maybe we can develop a more general specification what the semantics of the fixed-size types should be. I see an advantage in the writing part. These types ensure, that you always write the specified tag and integer width into the binary stream. This could help in supporting existing protocols. Shouldn't it then be the same for the reading part?
So the question for me currently is: Should an IntX value should always be translated from and to the MessagePack "int X" wire format?
There was a problem hiding this comment.
i think
Int means min is Int8 max Int64
Int8 means min is Int16 max Int64
Int16 means min is Int16 max Int64
Int32 means min is Int32 max Int64
maybe should add suffix "unsafe" or better one ?
Int8Unsafe means force Int8
Int16Unsafe means force Int16
Int32Unsafe means force Int32
There was a problem hiding this comment.
Ok, then let's come to the following decission: An IntX type should always be encoded to and decoded from an X bit value ignoring overflow scenarios. This means that we always use and assume the binary format of int X. A UintX type should behave equivalently with unsigned values.
Does this meet your requirements?
add sub type export
and a TypedObj