Re: Engine API naming
On Fri, 14 Dec 2012, Rob Godfrey wrote: On 14 December 2012 01:02, Weston M. Price wpr...@redhat.com wrote: On things such as bitmaps vs. enums, I think that's just a language convention thing... I don't see a huge need to make such things identical. Naming is something that should be aligned between APIs however. That was the point of the thread, not nit picking certain things. I'm not nit-picking - just trying to be being clear that I think some of the naming suggestions would actually make the API worse. I think drain() and update( .. ) give a truer sense of the intention of the API call (though if someone can suggest alternative names that given an even better sense of the action associated with the call, then I have no issue). Very much agreed! My suggestions are really just pointed ways of raising question. I appreciate the discussion. Justin
Re: Engine API naming
On Fri, 14 Dec 2012, Rafael Schloming wrote: On Thu, Dec 13, 2012 at 5:37 PM, Justin jr...@redhat.com wrote: I've found the whole process of proposing API review dispiriting. You can, of course, take it or leave it. I in no way wish to claim I have better choices. I only wish to point out things that might deserve more deliberation. Can you please elaborate on your comment about it being dispiriting? You've made several such remarks that have a distinctly negative connotation without providing enough material behind them for a constructive discourse. You say I haven't provided enough material for a constructive discourse. Really? What does a guy have to do? I've made a good faith effort to produce a document comparing the engine APIs in C and Java, and I've produced further revisions of it. It's more or less exactly the kind of thing one would use to hold a conversation in person or on the phone to review and discuss the naming. We've had multiple occasions to do so; we have so far declined. I've been present at many of those, and I know the general sentiment was ugh. I kept putting energy into it, and it kept meeting with little interest. That asymmetry is dispiriting. That's why I stopped and moved on to other problems. Justin
Re: Engine API naming
Comments below. On Fri, 14 Dec 2012, Rafael Schloming wrote: On Thu, Dec 13, 2012 at 5:37 PM, Justin jr...@redhat.com wrote: API usability is important and deserves attention. Take, for instance, DeliveryState versus Disposition. That only serves to confuse people. It's a difference that has no content. I also think link.drained and link.offered are very unclear. I've found the whole process of proposing API review dispiriting. You can, of course, take it or leave it. I in no way wish to claim I have better choices. I only wish to point out things that might deserve more deliberation. Justin --- Endpoint state PN_LOCAL_UNINIT Existing C name:PN_LOCAL_UNINIT Proposed C name:PN_LOCAL_UNINITIALIZED Existing Java name: EndpointState.UNINITIALIZED Proposed Java name: One api has the remote-local distinction in one bitset, and one does not. Is this difference desirable? The reason the C API has them in a single bitset is for clarity. There are numerous places where you need to pass in the local and/or remote state. If you separate these out into two bitsets, then you need two arguments everywhere, and it's difficult to remember which is the local state and which is the remote state, e.g.: pn_session_t *ssn = pn_session_head(connection, PN_UNINIT, PN_ACTIVE); vs pn_session_t *ssn = pn_session_head(connection, PN_LOCAL_UNINIT | PN_REMOTE_ACTIVE); It's also hypothetically a tad more extensible as we can add other state flags into the bit set in the future to support additional kinds of queries. PN_REMOTE_UNINIT Existing C name:PN_REMOTE_UNINIT Proposed C name:PN_REMOTE_UNINITIALIZED Existing Java name: EndpointState.UNINITIALIZED Proposed Java name: Personally, I'm at ease with UNINIT as an abbreviation here I'm confused by the comment. Are you suggesting a change here or not? My preference is to keep the C as is. I'm suggesting they should be the same, whichever way you go. Like you suggest, I'd be happy if Java went to EndpointState.UNINIT. (And I'd be just fine the other way too.) Types pn_disposition_t Existing C name:pn_disposition_t Proposed C name: Existing Java name: DeliveryState Proposed Java name: Disposition DeliveryState vs. Disposition. A good example of a confusing difference. I have a slight preference for DeliveryState (it's self- explanatory), but matching is the main thing. No one is saying anything about this one? pn_delivery_tag_t Existing C name:pn_delivery_tag_t Proposed C name: Existing Java name: [anonymous byte array] Proposed Java name: DeliveryTag? The C API also defines a pn_bytes_t which is almost identical to pn_delivery_tag_t: typedef struct pn_delivery_tag_t { size_t size; const char *bytes; } pn_delivery_tag_t; typedef struct { size_t size; char *start; } pn_bytes_t; The reason for the const is both so you can pass a const pointer to pn_delivery, and so that we can return a pointer to the bytes stored by the pn_delivery without worrying about users modifying them: pn_delivery_t *pn_delivery(pn_link_t *link, pn_delivery_tag_t tag); pn_delivery_tag_t pn_delivery_tag(pn_delivery_t *delivery); We could name the pn_delivery_tag_t something more generic though, e.g. pn_const_bytes_t or something like that. Session pn_session_connection Existing C name:pn_session_connection Proposed C name:pn_session_get_connection According to Rafi's system, this should have _get_. Why do you say this? There is no pn_session_set_connection. Allow me to explain. I thought you were reserving the _get_-less variants for dynamic/computed/derived slots. I went back and reread your statement, I think you meant a slightly broader set of things. To me, connection is a passive slot that just happens to get set once when you create the session. This kind of subtlety is why I don't much care for the distinction between _get_ and _get_-less attributes. I'd rather have one rule. Link pn_link_session Existing C name:pn_link_session Proposed C name:pn_link_get_session Same comment as above. Sender pn_link_drained Existing C name:pn_link_drained Proposed C name:pn_link_drain_credit [?] Existing Java name: Sender.drained Proposed Java name: Sender.drainCredit [?] I can't tell what's going on here. I particularly dislike pn_link_drained and _offered. They look like predicates but have no return value. These really need to be documented before a meaningful discussion can take place. Receiver pn_link_flow Existing C name:pn_link_flow Proposed C name:pn_link_issue_credit Existing Java name: Receiver.flow Proposed Java name: Receiver.issueCredit Consider pn_link_increase_credit, pn_link_issue_credit; working in the word credit somehow would help a lot Why do you think
[jira] [Closed] (PROTON-26) Engine api naming proposal
[ https://issues.apache.org/jira/browse/PROTON-26?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Rafael H. Schloming closed PROTON-26. - Resolution: Fixed Fix Version/s: 0.2 Assignee: Rafael H. Schloming I believe all the work for this has been done. I'm marking it closed, please reopen if there is more work. Engine api naming proposal -- Key: PROTON-26 URL: https://issues.apache.org/jira/browse/PROTON-26 Project: Qpid Proton Issue Type: Improvement Components: proton-c, proton-j Reporter: Justin Ross Assignee: Rafael H. Schloming Labels: api Fix For: 0.2 Attachments: engine-naming-01.patch, proton-engine-naming-proposal-2.pdf, proton-engine-naming-proposal-3.pdf See https://docs.google.com/spreadsheet/ccc?key=0ArGmVtK1EBOMdEw0bkp4OE5UOWpRRkx3RzVoTjliX0E#gid=0 -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PROTON-26) Engine api naming proposal
[ https://issues.apache.org/jira/browse/PROTON-26?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13531571#comment-13531571 ] Justin Ross commented on PROTON-26: --- That's really not the case. Rejecting it is fine, but it's mostly gone undiscussed. That's partly my fault. A post to the mailing list with highlights suitable for inline comments is incoming. Engine api naming proposal -- Key: PROTON-26 URL: https://issues.apache.org/jira/browse/PROTON-26 Project: Qpid Proton Issue Type: Improvement Components: proton-c, proton-j Reporter: Justin Ross Assignee: Rafael H. Schloming Labels: api Fix For: 0.2 Attachments: engine-naming-01.patch, proton-engine-naming-proposal-2.pdf, proton-engine-naming-proposal-3.pdf See https://docs.google.com/spreadsheet/ccc?key=0ArGmVtK1EBOMdEw0bkp4OE5UOWpRRkx3RzVoTjliX0E#gid=0 -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
Engine API naming
API usability is important and deserves attention. Take, for instance, DeliveryState versus Disposition. That only serves to confuse people. It's a difference that has no content. I also think link.drained and link.offered are very unclear. I've found the whole process of proposing API review dispiriting. You can, of course, take it or leave it. I in no way wish to claim I have better choices. I only wish to point out things that might deserve more deliberation. Justin --- Endpoint state PN_LOCAL_UNINIT Existing C name:PN_LOCAL_UNINIT Proposed C name:PN_LOCAL_UNINITIALIZED Existing Java name: EndpointState.UNINITIALIZED Proposed Java name: One api has the remote-local distinction in one bitset, and one does not. Is this difference desirable? PN_REMOTE_UNINIT Existing C name:PN_REMOTE_UNINIT Proposed C name:PN_REMOTE_UNINITIALIZED Existing Java name: EndpointState.UNINITIALIZED Proposed Java name: Personally, I'm at ease with UNINIT as an abbreviation here Types pn_disposition_t Existing C name:pn_disposition_t Proposed C name: Existing Java name: DeliveryState Proposed Java name: Disposition DeliveryState vs. Disposition. A good example of a confusing difference. I have a slight preference for DeliveryState (it's self- explanatory), but matching is the main thing. pn_delivery_tag_t Existing C name:pn_delivery_tag_t Proposed C name: Existing Java name: [anonymous byte array] Proposed Java name: DeliveryTag? Session pn_session_connection Existing C name:pn_session_connection Proposed C name:pn_session_get_connection According to Rafi's system, this should have _get_. Link pn_link_session Existing C name:pn_link_session Proposed C name:pn_link_get_session Sender pn_link_drained Existing C name:pn_link_drained Proposed C name:pn_link_drain_credit [?] Existing Java name: Sender.drained Proposed Java name: Sender.drainCredit [?] I can't tell what's going on here. I particularly dislike pn_link_drained and _offered. They look like predicates but have no return value. Receiver pn_link_flow Existing C name:pn_link_flow Proposed C name:pn_link_issue_credit Existing Java name: Receiver.flow Proposed Java name: Receiver.issueCredit Consider pn_link_increase_credit, pn_link_issue_credit; working in the word credit somehow would help a lot pn_link_drain Existing C name:pn_link_drain Proposed C name:pn_link_rescind_credit Existing Java name: Receiver.drain Proposed Java name: Receiver.rescindCredit Consider pn_link_decrease_credit, pn_link_rescind_credit Delivery pn_delivery_link Existing C name:pn_delivery_link Proposed C name:pn_delivery_get_link pn_delivery_local_state Existing C name:pn_delivery_local_state Proposed C name:pn_delivery_state Existing Java name: Delivery.getLocalState Proposed Java name: Delivery.state Local is not used elsewhere for local/remote distinctions pn_delivery_update Existing C name:pn_delivery_update Proposed C name:pn_delivery_acknowledge Existing Java name: Delivery.disposition Proposed Java name: Delivery.acknowledge Do calls to delivery.update correspond one-to-one to delivery.updated? The naming implies a symmetry that I'm not sure is there. On Thu, 13 Dec 2012, Justin Ross (JIRA) wrote: [ https://issues.apache.org/jira/browse/PROTON-26?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13531571#comment-13531571 ] Justin Ross commented on PROTON-26: --- That's really not the case. Rejecting it is fine, but it's mostly gone undiscussed. That's partly my fault. A post to the mailing list with highlights suitable for inline comments is incoming. Engine api naming proposal -- Key: PROTON-26 URL: https://issues.apache.org/jira/browse/PROTON-26 Project: Qpid Proton Issue Type: Improvement Components: proton-c, proton-j Reporter: Justin Ross Assignee: Rafael H. Schloming Labels: api Fix For: 0.2 Attachments: engine-naming-01.patch, proton-engine-naming-proposal-2.pdf, proton-engine-naming-proposal-3.pdf See https://docs.google.com/spreadsheet/ccc?key=0ArGmVtK1EBOMdEw0bkp4OE5UOWpRRkx3RzVoTjliX0E#gid=0 -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: Engine API naming
Nice suggestions actually. I think some of the discrepancy has to do with the difference between languages and the idioms that have long been fossilized int the different naming conventions. Again, all suggestions on proposed look good to me. Regards, -W On Dec 13, 2012, at 5:37 PM, Justin jr...@redhat.com wrote: API usability is important and deserves attention. Take, for instance, DeliveryState versus Disposition. That only serves to confuse people. It's a difference that has no content. I also think link.drained and link.offered are very unclear. I've found the whole process of proposing API review dispiriting. You can, of course, take it or leave it. I in no way wish to claim I have better choices. I only wish to point out things that might deserve more deliberation. Justin --- Endpoint state PN_LOCAL_UNINIT Existing C name:PN_LOCAL_UNINIT Proposed C name:PN_LOCAL_UNINITIALIZED Existing Java name: EndpointState.UNINITIALIZED Proposed Java name: One api has the remote-local distinction in one bitset, and one does not. Is this difference desirable? PN_REMOTE_UNINIT Existing C name:PN_REMOTE_UNINIT Proposed C name:PN_REMOTE_UNINITIALIZED Existing Java name: EndpointState.UNINITIALIZED Proposed Java name: Personally, I'm at ease with UNINIT as an abbreviation here Types pn_disposition_t Existing C name:pn_disposition_t Proposed C name: Existing Java name: DeliveryState Proposed Java name: Disposition DeliveryState vs. Disposition. A good example of a confusing difference. I have a slight preference for DeliveryState (it's self- explanatory), but matching is the main thing. pn_delivery_tag_t Existing C name:pn_delivery_tag_t Proposed C name: Existing Java name: [anonymous byte array] Proposed Java name: DeliveryTag? Session pn_session_connection Existing C name:pn_session_connection Proposed C name:pn_session_get_connection According to Rafi's system, this should have _get_. Link pn_link_session Existing C name:pn_link_session Proposed C name:pn_link_get_session Sender pn_link_drained Existing C name:pn_link_drained Proposed C name:pn_link_drain_credit [?] Existing Java name: Sender.drained Proposed Java name: Sender.drainCredit [?] I can't tell what's going on here. I particularly dislike pn_link_drained and _offered. They look like predicates but have no return value. Receiver pn_link_flow Existing C name:pn_link_flow Proposed C name:pn_link_issue_credit Existing Java name: Receiver.flow Proposed Java name: Receiver.issueCredit Consider pn_link_increase_credit, pn_link_issue_credit; working in the word credit somehow would help a lot pn_link_drain Existing C name:pn_link_drain Proposed C name:pn_link_rescind_credit Existing Java name: Receiver.drain Proposed Java name: Receiver.rescindCredit Consider pn_link_decrease_credit, pn_link_rescind_credit Delivery pn_delivery_link Existing C name:pn_delivery_link Proposed C name:pn_delivery_get_link pn_delivery_local_state Existing C name:pn_delivery_local_state Proposed C name:pn_delivery_state Existing Java name: Delivery.getLocalState Proposed Java name: Delivery.state Local is not used elsewhere for local/remote distinctions pn_delivery_update Existing C name:pn_delivery_update Proposed C name:pn_delivery_acknowledge Existing Java name: Delivery.disposition Proposed Java name: Delivery.acknowledge Do calls to delivery.update correspond one-to-one to delivery.updated? The naming implies a symmetry that I'm not sure is there. On Thu, 13 Dec 2012, Justin Ross (JIRA) wrote: [ https://issues.apache.org/jira/browse/PROTON-26?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13531571#comment-13531571 ] Justin Ross commented on PROTON-26: --- That's really not the case. Rejecting it is fine, but it's mostly gone undiscussed. That's partly my fault. A post to the mailing list with highlights suitable for inline comments is incoming. Engine api naming proposal -- Key: PROTON-26 URL: https://issues.apache.org/jira/browse/PROTON-26 Project: Qpid Proton Issue Type: Improvement Components: proton-c, proton-j Reporter: Justin Ross Assignee: Rafael H. Schloming Labels: api Fix For: 0.2 Attachments: engine-naming-01.patch, proton-engine-naming-proposal-2.pdf, proton-engine-naming-proposal-3.pdf See https://docs.google.com/spreadsheet/ccc?key
Re: Engine API naming
On Dec 13, 2012, at 7:23 PM, Rob Godfrey rob.j.godf...@gmail.com wrote: On 14 December 2012 01:02, Weston M. Price wpr...@redhat.com wrote: On Dec 13, 2012, at 6:22 PM, Rob Godfrey rob.j.godf...@gmail.com wrote: A couple of comments... On 13 December 2012 23:37, Justin jr...@redhat.com wrote: API usability is important and deserves attention. snip pn_link_drain Existing C name:pn_link_drain Proposed C name:pn_link_rescind_credit Existing Java name: Receiver.drain Proposed Java name: Receiver.rescindCredit Consider pn_link_decrease_credit, pn_link_rescind_credit Drain *doesn't* rescind credit, or decrease credit, so I'd be -1 on these names. Drain is an indication that the sender should use all available credit, but if insufficient deliveries are available at the sender to use up all the credit, only then should it act as if all credit had been consumed. At no point is the receiver rescinding credit. snip pn_delivery_update Existing C name:pn_delivery_update Proposed C name:pn_delivery_acknowledge Existing Java name: Delivery.disposition Proposed Java name: Delivery.acknowledge Do calls to delivery.update correspond one-to-one to delivery.updated? The naming implies a symmetry that I'm not sure is there. I'm -1 on acknowledged. Acknowledgement is one type of update, but not the only one. I'm fine with changing the Java to update() to match the C. On things such as bitmaps vs. enums, I think that's just a language convention thing... I don't see a huge need to make such things identical. Naming is something that should be aligned between APIs however. That was the point of the thread, not nit picking certain things. I'm not nit-picking - just trying to be being clear that I think some of the naming suggestions would actually make the API worse. I think drain() and update( .. ) give a truer sense of the intention of the API call (though if someone can suggest alternative names that given an even better sense of the action associated with the call, then I have no issue). Easy dude...decaf perhaps? I was simply agreeing with Justin about the discrepancy between the API names in the varying implementations. I don't know anything about intention or anything else. I assume you guys already have that figured out don't you? I just work here.
[jira] [Updated] (PROTON-26) Engine api naming proposal
[ https://issues.apache.org/jira/browse/PROTON-26?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Justin Ross updated PROTON-26: -- Component/s: proton-j Engine api naming proposal -- Key: PROTON-26 URL: https://issues.apache.org/jira/browse/PROTON-26 Project: Qpid Proton Issue Type: Improvement Components: proton-c, proton-j Reporter: Justin Ross Attachments: engine-naming-01.patch, proton-engine-naming-proposal-2.pdf, proton-engine-naming-proposal-3.pdf See https://docs.google.com/spreadsheet/ccc?key=0ArGmVtK1EBOMdEw0bkp4OE5UOWpRRkx3RzVoTjliX0E#gid=0 -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PROTON-26) Engine api naming proposal
[ https://issues.apache.org/jira/browse/PROTON-26?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Justin Ross updated PROTON-26: -- Attachment: proton-engine-naming-proposal-3.pdf Attached an updated proposal with Rafi's comments and recent changes factored in. Engine api naming proposal -- Key: PROTON-26 URL: https://issues.apache.org/jira/browse/PROTON-26 Project: Qpid Proton Issue Type: Improvement Components: proton-c, proton-j Reporter: Justin Ross Attachments: engine-naming-01.patch, proton-engine-naming-proposal-2.pdf, proton-engine-naming-proposal-3.pdf See https://docs.google.com/spreadsheet/ccc?key=0ArGmVtK1EBOMdEw0bkp4OE5UOWpRRkx3RzVoTjliX0E#gid=0 -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: Proton engine api naming proposal
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. Justin
Re: Proton engine api naming proposal
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. 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. 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. 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). 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. -- Rob Justin
[jira] [Commented] (PROTON-26) Engine api naming proposal
[ https://issues.apache.org/jira/browse/PROTON-26?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13468498#comment-13468498 ] Rafael H. Schloming commented on PROTON-26: --- The reason the transport uses input/output rather than read/write is because read/write would have exactly the opposite of the usual semantics with respect to its arguments. A read operation typically populates the passed in array with data, whereas the input operation is actually providing input in the passed in array that is consumed by the transport. Likewise a write operation usually consumes/copies bytes from the passed in array whereas the output operation is actually filling the passed in array with output from the transport. Reversing the names (input - write, output - read) to make the semantics similar with respect to the arguments would also be confusing as you'd be calling Transport.read in order to produce output to write to the network. As it is now it's pretty easy to remember that you read from the network to provide input to the transport and when the transport supplies output, you write to the network. Engine api naming proposal -- Key: PROTON-26 URL: https://issues.apache.org/jira/browse/PROTON-26 Project: Qpid Proton Issue Type: Improvement Components: proton-c Reporter: Justin Ross Attachments: engine-naming-01.patch, proton-engine-naming-proposal-2.pdf See https://docs.google.com/spreadsheet/ccc?key=0ArGmVtK1EBOMdEw0bkp4OE5UOWpRRkx3RzVoTjliX0E#gid=0 -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: Proton engine api naming proposal
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? Justin
Re: Proton engine api naming proposal
On Wed, Oct 03, 2012 at 08:35:00AM -0400, 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(). Agreed. The above APIs were a legacy of the pre-standardized APIs IIANM. 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? +1 I find APIs of the form pn_verb_object to be extremely expressive, making the source code much more readable. -- Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc. Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ pgp1c7v06PB1A.pgp Description: PGP signature
[jira] [Created] (PROTON-26) Engine api naming proposal
Justin Ross created PROTON-26: - Summary: Engine api naming proposal Key: PROTON-26 URL: https://issues.apache.org/jira/browse/PROTON-26 Project: Qpid Proton Issue Type: Improvement Components: proton-c Reporter: Justin Ross -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: Proton engine api naming proposal
On Thu, Sep 13, 2012 at 9:36 AM, Justin Ross jr...@redhat.com wrote: On Thu, 13 Sep 2012, Ted Ross wrote: I'm not crazy about the work-processing function names as they seem to disregard the grammar. Should they not all be pn_connection_* functions? I agree about this. I would definitely prefer to see pn_connection_* for the connection-scoped work interfaces. I guess I thought I was pressing my luck, :). I think there actually already is a consistent rule here, it's just missing from your grammar. Wherever there is a linked list of things, the API uses the form: pn_collection_head(pn_root_t) pn_collection_next(pn_element_t_t) I think this is better than trying to stick it all on the root or all on the element or splitting it up between the two. For example I think pn_work_head is better than pn_head_delivery as the latter gives you less information. The fact that it is a delivery is already contained in the type signature, and there are multiple lists of deliveries maintained by the engine, so just knowing that it is a list of deliveries isn't sufficient. Even scoping it to the connection is not terribly useful as there may well be multiple lists of deliveries on the connection. The relevant info here is that it is the head of the work queue, a concept that we actually do (or should) explain at length (somewhere). I would argue that the work queue is actually the relevant concept/noun here, it just doesn't have it's own lifecycle since it is a component of the connection. In Java or some other garbage collected language you might see the noun expressed directly as an object, e.g.: WorkIterator work = new WorkIterator(connection); while (...) { Delivery d = work.next(); } This exact pattern of course is extremely cumbersome and inefficient in C since you'd have to malloc an object just to iterate, so naturally you use a linked list instead, but I think conceptually the noun still exists and if we lose the noun from the name we are missing an important key to index into the documentation. --Rafael
Re: Proton engine api naming proposal
- Original Message - On Thu, Sep 13, 2012 at 9:36 AM, Justin Ross jr...@redhat.com wrote: On Thu, 13 Sep 2012, Ted Ross wrote: I'm not crazy about the work-processing function names as they seem to disregard the grammar. Should they not all be pn_connection_* functions? I agree about this. I would definitely prefer to see pn_connection_* for the connection-scoped work interfaces. I guess I thought I was pressing my luck, :). I think there actually already is a consistent rule here, it's just missing from your grammar. Wherever there is a linked list of things, the API uses the form: pn_collection_head(pn_root_t) pn_collection_next(pn_element_t_t) I think this is better than trying to stick it all on the root or all on the element or splitting it up between the two. For example I think pn_work_head is better than pn_head_delivery as the latter gives you less information. The fact that it is a delivery is already contained in the type signature, and there are multiple lists of deliveries maintained by the engine, so just knowing that it is a list of deliveries isn't sufficient. Even scoping it to the connection is not terribly useful as there may well be multiple lists of deliveries on the connection. The relevant info here is that it is the head of the work queue, a concept that we actually do (or should) explain at length (somewhere). I would argue that the work queue is actually the relevant concept/noun here, it just doesn't have it's own lifecycle since it is a component of the connection. In Java or some other garbage collected language you might see the noun expressed directly as an object, e.g.: WorkIterator work = new WorkIterator(connection); while (...) { Delivery d = work.next(); } This exact pattern of course is extremely cumbersome and inefficient in C since you'd have to malloc an object just to iterate, so naturally you use a linked list instead, but I think conceptually the noun still exists and if we lose the noun from the name we are missing an important key to index into the documentation. --Rafael All great stuff. I guess I'm surprised there aren't at least 2 standard C coding styles out there to choose from. So from now on I'll refer to this style as Rossrafi. Dude, I can't believe you're coding C like that. Why aren't you using Rossrafi? William