On Thu, Oct 4, 2012 at 6:31 PM, Rob Godfrey <rob.j.godf...@gmail.com> wrote:

> On 4 October 2012 23:56, Justin Ross <jr...@redhat.com> wrote:
> > On Wed, 3 Oct 2012, Justin wrote:
> >
> >> On Wed, 3 Oct 2012, Rafael Schloming wrote:
> >>
> >>> I believe the convention I'm following is actually the norm (for a good
> >>> reason). The get/set_foo
> >>> pattern is used for passive slots, i.e. it's a strong signal that if
> you
> >>> call set_foo with a given
> >>> value then get_foo will return that same value until set_foo is called
> >>> again. Whereas
> >>> dynamic/computed/derived values (something where it would never make
> >>> sense to have a setter) are
> >>> generally not prefixed by get. Some examples in Java would be things
> like
> >>> Collection.size(),
> >>> Object.hashCode(), Map.values(). I think this is a pretty valuable
> >>> convention as it is a signal that
> >>
> >>
> >> I agree that's a common convention in java.  It's not "the norm":
> >> counterexamples are Thread.getState(), Integer.getInteger(s),
> >> File.getFreeSpace().
> >>
> >> In any case, it's arguably a good convention.  It has one particular
> >> practical problem, more collisions.  This problem is exhibited right
> now in
> >> pn_link_drained.  What does that do?  It *looks like* it is a dynamic
> >> predicate, but it isn't.  If in the future you want to add such a
> predicate,
> >> you'll have a collision.  _get_ keeps things cleanly separated. (In the
> case
> >> of pn_link_drained, I think that just needs a better name.)
> >>
> >> I'm pleased we're discussing this.  Can we discuss it (and all the other
> >> things worth discussing, imo) before we set down changes in the code?
> >
> >
> > I've updated the proposal again.  I've also adjusted the Java naming to
> > match the C naming in the main.  Of course, they needn't work the same
> > necessarily, but for instance, I figured that if we're going to
> distinguish
> > between computed/derived and passive attributes in C, we ought to as
> well in
> > Java.
> >
>
> So... personally I don't have quite the horror of extra characters
> that certain others do... and I think prepending an "is" on methods
> that are returning a boolean is a reasonable deviation from the
> pattern established in the C.
>

The C actually does follow the is_ convention, e.g.:

  bool pn_message_is_durable(pn_message_t *msg)
  int pn_message_set_durable(pn_message_t *msg, bool durable)

I believe the only place where it deviates from this is for dynamically
computed values like. pn_delivery_writable() which is consistent with the
convention I described before regarding dynamically computed values.

I can buy that there are certain methods where the get/set pairing
> doesn't really imply the actual semantics that the methods have.  In
> general for the "count" type variables that cannot directly be set, I
> think getXxxCount() also works, but I'm also happy with the current
> xxx() style.
>

My problem with the getXxxCount() pattern is that it doesn't really fit
with all the terms, e.g. getCreditCount() doesn't really make sense. It
would have to be something more like getPotentialDeliveryCount(), but even
this wouldn't be sufficient as there is more than one potential delivery
count associated with a link (e.g. available vs credit).

In general beyond the basic conventional stuff which seems fairly
uncontroversial, I think we actually need more documentation to properly
motivate and discuss naming changes. Names really are a
mnemonic/pointer/key into a mental model, and it really helps to have both
a shared and accurate mental model when discussing them. Flow control in
particular is a fairly complex domain that requires keeping multiple
time-lagged viewpoints in your head as they exchange numerous formally
defined quantities. Without a solid mental model pretty much any names will
seem opaque and useless. To use an analogy, getting every math library in
the world and renaming the sine function to getOppositeToHypotenuseRatio,
isn't really going to make anyone understand trigonometry. Of course I
realize there is a tradeoff here, but I think one of the better ways to
figure out where that tradeoff lies is to actually document the concepts as
then it becomes clearer how a name will impact the total economy of
expression.


> There are a couple of areas where I think the naming definitely needs
> work, in particular hostname: The "local" hostname is actually the
> hostname that is desired to be chosen at the remote endpoint, while
> the "remote" hostname is the hostname that the remote endpoint desires
> at the local side.
>

The C code uses a slightly different convention for local/remote stuff and
in this case it actually helps a little, rather than doing local_foo and
remote_foo it does foo and remote_foo. So at least we don't end up with
something called local_hostname that doesn't actually refer to the local
hostname, however the same problem is there for the remote hostname.

Of the proposals, I think Delivery.update is less wrong than
> "acknowledge" ... the update may not be an acknowledgement but some
> other form of state change. Also rescind_credit is not really the
> semantics of drain (if there are messages available then the peer
> should send them, not simply cancel the credit).
>

+1

Source and Target - currently the API is incomplete here - a proper
> definition of these types is needed which is why in the Java I've
> currently got these as Source/Target Address.  The Source and Target
> types should be properly defined with APIs which allow for the setting
> of other attributes than just the address.
>

This definitely needs to change. One of the complications here is that the
transactional spec actually defines completely different sources/targets,
so it's not as simple as just defining the specific fields in the messaging
specification, we need some way to swap in an entirely different kind of
source/target.

--Rafael

Reply via email to