Re: messenger accessors
Unless I'm missing something, I don't think this pattern would work in the
general case. For example it's valid to set a timeout of zero, so you'd
either be stuck with no way to get a timeout without setting it, or you'd
have no way to set it back to zero once you'd set it to something else.
There's also some benefit to keeping the accessor pattern simple when it
comes to using swig to produce bindings.
--Rafael
On Sat, Feb 9, 2013 at 6:34 AM, Michael Goulish mgoul...@redhat.com wrote:
Most of the Messenger interface is accessor functions -- 53% --
19 functions out of a total of 39 .
16 of those have both 'get' and 'set', while 3 are get-only.
Those 3 do not have the word 'get' in their names, so you have
to look at code to realize that they are accessors like the others.
Could we consider single accessors for everything ?
This would require that all enumtypes use 0 to mean not a value.
( It has always seemed to me that this is a good policy anyway.
You always need that non-value sooner or later. )
This change would reduce the size of the messenger interface by a
large fraction -- 9 fns, which seems like a good thing,
and would reduce the fraction of simple accessors to one-third of
the interface. It would also unify the naming convention.
( Accessors that must be get-only would simply not take an input,
as now. )
typedef enum {
PN_ACCEPT_MODE_NONE,
PN_ACCEPT_MODE_AUTO,
PN_ACCEPT_MODE_MANUAL
} pn_accept_mode_t;
pn_accept_mode_t
pn_messenger_get_accept_mode ( pn_messenger_t * messenger,
pn_accept_mode_t mode
)
{
if ( mode )
messenger-accept_mode = mode;
return messenger-accept_mode;
}
Re: messenger accessors
Sorry, I only thought of pointers and enumtypes.
To take care of integer variables that allow zero, we could use a pattern like
this:
( Would this work better for swig? )
pn_whatever_t
pn_messenger_whatever ( pn_messenger_t * messenger,
pn_whatever_t whatever
)
{
if ( ! PN_WHATEVER_NON_VALUE(whatever) )
messenger-whatever = whatever;
return messenger-whatever;
}
Then you get to define non value explicitly for each type.
The only place this won't work, then is i.e. a float value that allows the
whole numberline, but I bet we won't have any of those.
- Original Message -
From: Rafael Schloming r...@alum.mit.edu
To: proton@qpid.apache.org
Sent: Monday, February 11, 2013 8:50:54 AM
Subject: Re: messenger accessors
Unless I'm missing something, I don't think this pattern would work in the
general case. For example it's valid to set a timeout of zero, so you'd
either be stuck with no way to get a timeout without setting it, or you'd
have no way to set it back to zero once you'd set it to something else.
There's also some benefit to keeping the accessor pattern simple when it
comes to using swig to produce bindings.
--Rafael
On Sat, Feb 9, 2013 at 6:34 AM, Michael Goulish mgoul...@redhat.com wrote:
Most of the Messenger interface is accessor functions -- 53% --
19 functions out of a total of 39 .
16 of those have both 'get' and 'set', while 3 are get-only.
Those 3 do not have the word 'get' in their names, so you have
to look at code to realize that they are accessors like the others.
Could we consider single accessors for everything ?
This would require that all enumtypes use 0 to mean not a value.
( It has always seemed to me that this is a good policy anyway.
You always need that non-value sooner or later. )
This change would reduce the size of the messenger interface by a
large fraction -- 9 fns, which seems like a good thing,
and would reduce the fraction of simple accessors to one-third of
the interface. It would also unify the naming convention.
( Accessors that must be get-only would simply not take an input,
as now. )
typedef enum {
PN_ACCEPT_MODE_NONE,
PN_ACCEPT_MODE_AUTO,
PN_ACCEPT_MODE_MANUAL
} pn_accept_mode_t;
pn_accept_mode_t
pn_messenger_get_accept_mode ( pn_messenger_t * messenger,
pn_accept_mode_t mode
)
{
if ( mode )
messenger-accept_mode = mode;
return messenger-accept_mode;
}
Re: messenger accessors
I don't think this will work in all cases, e.g. get/set_incoming_window is
a normal int, not a typedef, and in fact set_incoming_window returns an
error code if the value passed in is invalid. With the signature you're
suggesting there would be no way for a setter to indicate an error.
--Rafael
On Mon, Feb 11, 2013 at 9:14 AM, Michael Goulish mgoul...@redhat.comwrote:
Sorry, I only thought of pointers and enumtypes.
To take care of integer variables that allow zero, we could use a pattern
like this:
( Would this work better for swig? )
pn_whatever_t
pn_messenger_whatever ( pn_messenger_t * messenger,
pn_whatever_t whatever
)
{
if ( ! PN_WHATEVER_NON_VALUE(whatever) )
messenger-whatever = whatever;
return messenger-whatever;
}
Then you get to define non value explicitly for each type.
The only place this won't work, then is i.e. a float value that allows the
whole numberline, but I bet we won't have any of those.
- Original Message -
From: Rafael Schloming r...@alum.mit.edu
To: proton@qpid.apache.org
Sent: Monday, February 11, 2013 8:50:54 AM
Subject: Re: messenger accessors
Unless I'm missing something, I don't think this pattern would work in the
general case. For example it's valid to set a timeout of zero, so you'd
either be stuck with no way to get a timeout without setting it, or you'd
have no way to set it back to zero once you'd set it to something else.
There's also some benefit to keeping the accessor pattern simple when it
comes to using swig to produce bindings.
--Rafael
On Sat, Feb 9, 2013 at 6:34 AM, Michael Goulish mgoul...@redhat.com
wrote:
Most of the Messenger interface is accessor functions -- 53% --
19 functions out of a total of 39 .
16 of those have both 'get' and 'set', while 3 are get-only.
Those 3 do not have the word 'get' in their names, so you have
to look at code to realize that they are accessors like the others.
Could we consider single accessors for everything ?
This would require that all enumtypes use 0 to mean not a value.
( It has always seemed to me that this is a good policy anyway.
You always need that non-value sooner or later. )
This change would reduce the size of the messenger interface by a
large fraction -- 9 fns, which seems like a good thing,
and would reduce the fraction of simple accessors to one-third of
the interface. It would also unify the naming convention.
( Accessors that must be get-only would simply not take an input,
as now. )
typedef enum {
PN_ACCEPT_MODE_NONE,
PN_ACCEPT_MODE_AUTO,
PN_ACCEPT_MODE_MANUAL
} pn_accept_mode_t;
pn_accept_mode_t
pn_messenger_get_accept_mode ( pn_messenger_t * messenger,
pn_accept_mode_t mode
)
{
if ( mode )
messenger-accept_mode = mode;
return messenger-accept_mode;
}
