Re: [PATCH 3/3] [MAMAC] mamaPublisherImpl_getTransportImpl() accessor


Alireza Assadzadeh <Alireza.Assadzadeh@...>
 

Hi Gary,

Thanks for the patch. I had some minor comments which I marked inline below with [AA:]

--Alireza

-----Original Message-----
From: openmama-dev-bounces@lists.openmama.org [mailto:openmama-
dev-bounces@lists.openmama.org] On Behalf Of Gary Molloy
Sent: Thursday, April 23, 2015 12:54 PM
To: openmama-dev@lists.openmama.org
Subject: [Openmama-dev] [PATCH 3/3] [MAMAC]
mamaPublisherImpl_getTransportImpl() accessor

[snip]

Added a new mamaPublisherImpl_getTransportImpl() accessor method
that will return the mamaTransport from the mamaPublisher.
[AA:] I suggest dropping the 'Impl' in the new function name and making it part of the public API. So, calling it mamaPublisher_getTransport.

Signed-off-by: Gary Molloy <g.molloy@srtechlabs.com>
---
mama/c_cpp/src/c/publisher.c     |    7 +++++++
mama/c_cpp/src/c/publisherimpl.h |    3 +++
2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/mama/c_cpp/src/c/publisher.c b/mama/c_cpp/src/c/publisher.c
index 281b054..e09017c 100644
--- a/mama/c_cpp/src/c/publisher.c
+++ b/mama/c_cpp/src/c/publisher.c
@@ -600,4 +600,11 @@ mama_status mamaPublisherImpl_clearTransport
(mamaPublisher publisher)
     return mamaPublisherImpl_destroy((mamaPublisherImpl *)publisher);
}

+mamaTransport
+mamaPublisherImpl_getTransportImpl (mamaPublisher publisher)
+{
+    mamaPublisherImpl* impl   = (mamaPublisherImpl*)publisher;

+    if (!impl) return NULL;
+    return impl->mTport;
+}
[AA:] Can the symantics be changed to the following instead?

MAMAExpDLL
extern mama_status
mamaSubscription_getTransport (
mamaPublisher publisher,
mamaTransport *transport);

This makes it also consistent with the similar function mamaSubscription_getTransport

diff --git a/mama/c_cpp/src/c/publisherimpl.h
b/mama/c_cpp/src/c/publisherimpl.h
index f336fd8..b9b9c43 100644
--- a/mama/c_cpp/src/c/publisherimpl.h
+++ b/mama/c_cpp/src/c/publisherimpl.h
@@ -44,6 +44,9 @@ mamaPublisher_sendFromInboxByIndex
(mamaPublisher publisher,

 mama_status mamaPublisherImpl_clearTransport (mamaPublisher
publisher);

+extern mamaTransport
+mamaPublisherImpl_getTransportImpl (mamaPublisher publisher);
+
[AA:] I suggest moving the function declaration to the public API header mama/c_cpp/src/c/mama/publisher.h instead of mama/c_cpp/src/c/publisherimpl.h.

The function can be generally useful to the API users. On the subscription side we have the mamaSubscription_getTransport which is similar.

[snip]

Join Openmama-dev@lists.openmama.org to automatically receive all group messages.