TooTallNate / ref-struct

Create ABI-compliant "struct" instances on top of Buffers
118 stars 78 forks source link

Struct should not rely on object key order #6

Closed kmsquire closed 10 years ago

kmsquire commented 10 years ago

As noted here: http://stackoverflow.com/questions/9179680/is-it-acceptable-style-for-node-js-libraries-to-rely-on-object-key-order

This is somewhat pedantic, but the code will break if it's ever run on a javascript engine that does not keep track of key order.

kmsquire commented 10 years ago

Of course, after writing this, I'm not sure how the following would work:

var PasswordEntry = Struct({
    'username': 'string'
  , 'password': 'string'
  , 'salt':     int
})
TooTallNate commented 10 years ago

I'd be open to an alternative syntax that's more future-proof, but I'm not overly worried about it since, as you've already pointed out, ref-struct only runs on node (though I could see that changing some day in the future).

parasyte commented 10 years ago

Following in the footsteps of Python's OrderedDict, the keys would be specified as an array of arrays:

var PasswordEntry = Struct([
    [ "username", "string" ],
    [ "password", "string" ],
    [ "salt", "int" ]
]);
TooTallNate commented 10 years ago

@parasyte +1, I love it :)

TooTallNate commented 10 years ago

@parasyte @kmsquire So I actually forgot about it since there's no tests or examples, but there's actually a "legacy API" implemented in ref-struct. It's identical to @parasyte's proposed syntax, however, the type and name values are inverted. That is, you put the type first and the name second, kinda resembling actual C syntax.

I believe that this covers the needs of this issue enough that we can close. Cheers!

parasyte commented 10 years ago

:+1: Got it.

For anyone else who stumbles upon this ticket in the future: https://github.com/TooTallNate/ref-struct/blob/19af92d25c40bcfdb36c060f71833d363ccac790/lib/struct.js#L131-L136

socketpair commented 9 years ago

Why not to remove legacy API ? At least write to console deprecation warning for some time.

socketpair commented 9 years ago

So, Please change example in README.md that will show NEW API (use Array instaed of Object), and, if you want, legacy API. I think legacy API should not be show in examples since it become legacy.

socketpair commented 9 years ago

ARGHHH. you think that Array-API is alternative, not Main API. :( well, no changes is required so.

TooTallNate commented 9 years ago

I don't think either one is legacy. I shouldn't have used that word before. They're just alternative syntaxes.

I'll take a pull request showing both styles in the README :wink: