confused parameter order asking for trouble
Nicholas Clark
void Parrot_PMC_set_intval_intkey(Parrot_INTERP interp, Parrot_PMC pmc, Parrot_Int value, Parrot_Int key) {
VTABLE_set_integer_keyed_int(interp, pmc, key, value);
}
Is there any reason why the vtable is key, value but the extension interface
is value, key? This parameter transposition strikes me as asking for trouble.
I'll patch everything in core to make value last if consensus is that this
is the right thing to do.
Nicholas Clark
Leopold Toetsch
> I've just fallen into this trap, and I doubt I'll be the last one:
> void Parrot_PMC_set_intval_intkey(Parrot_INTERP interp, Parrot_PMC pmc, Parrot_Int value, Parrot_Int key) {
> VTABLE_set_integer_keyed_int(interp, pmc, key, value);
>}
> Is there any reason why the vtable is key, value but the extension
> interface is value, key? This parameter transposition strikes me as
> asking for trouble.
It seems order was chosen according to function name (_intval _intkey).
> I'll patch everything in core to make value last if consensus is that
> this is the right thing to do.
Yep. I'd swap function names as well as argument order, so that
everything matches the vtable prototype.
> Nicholas Clark
leo
Nicholas Clark
I'm not sure that swapping all the function names is great. The vtable
currently has entries such as
INTVAL get_integer()
INTVAL get_integer_keyed(PMC* key)
INTVAL get_integer_keyed_int(INTVAL key)
INTVAL get_integer_keyed_str(STRING* key)
void set_integer_native(INTVAL value)
void set_integer_same(PMC* value)
void set_integer_keyed(PMC* key, INTVAL value)
void set_integer_keyed_int(INTVAL key, INTVAL value)
void set_integer_keyed_str(STRING* key, INTVAL value)
^key type
^variant
^slot type
^action
and the rename change would turn
void Parrot_PMC_set_intval_intkey(Parrot_INTERP interp, Parrot_PMC pmc, Parrot_Int value, Parrot_Int key)
to
void Parrot_PMC_set_intkey_intval(Parrot_INTERP interp, Parrot_PMC pmc, Parrot_Int value, Parrot_Int key)
^slot type
^key type and variant
^action
which isn't the same order
This may be because the current order (action, slot type, variant/key type)
followed by value reflects the way my brain orders the importance of things.
Nicholas Clark
Leopold Toetsch
> On Fri, Jun 25, 2004 at 09:46:53AM +0200, Leopold Toetsch wrote:
>>
>> Yep. I'd swap function names as well as argument order, so that
>> everything matches the vtable prototype.
> void Parrot_PMC_set_intkey_intval(Parrot_INTERP interp, Parrot_PMC
> pmc, Parrot_Int value, Parrot_Int key)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"as well as argument order"
But I don't care, as long as vtables remain ;)
> Nicholas Clark
leo
Nicholas Clark
> Nicholas Clark <ni...@ccl4.org> wrote:
> > On Fri, Jun 25, 2004 at 09:46:53AM +0200, Leopold Toetsch wrote:
> >>
> >> Yep. I'd swap function names as well as argument order, so that
> >> everything matches the vtable prototype.
>
>
> > void Parrot_PMC_set_intkey_intval(Parrot_INTERP interp, Parrot_PMC
> > pmc, Parrot_Int value, Parrot_Int key)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> "as well as argument order"
D'oh. Silly me.
> But I don't care, as long as vtables remain ;)
I committed the change I suggested (change parameter order so that value
is always last) and then committed the change to ponie to keep it in sync,
and then realised I'd not checked the entire parrot source tree for any
other instances of these functions.
docs/pdds/pdd11_extending.pod:7
Er. I'm changing the design here. I guess I had better ask Dan - is this
OK?
Nicholas Clark
Leopold Toetsch
> ... committed the change to ponie to keep it in sync,
> and then realised I'd not checked the entire parrot source tree for any
> other instances of these functions.
extend.h isn't included inside Parrot. I don't know, if there are any
embedders/extenders except ponie.
> docs/pdds/pdd11_extending.pod:7
> Er. I'm changing the design here. I guess I had better ask Dan - is this
> OK?
I'm not doing that "surface policy" stuff, and I try hard not to
touch pdds[1].
But given that above (silent) assumption is true, just change the
prototypes. It's an editor s/from/to/ command (or sequence), which you
can provide in your change notes for people that *are* using extend.h ;)
> Nicholas Clark
leo
[1] well, tossing tons of <binop>_keyed functions *was* fun :)
Dan Sugalski
Yes. Fix up the PDD, though, to match the change.
Dan
--------------------------------------"it's like this"-------------------
Dan Sugalski even samurai
d...@sidhe.org have teddy bears and even
teddy bears get drunk