Skip to content

Conversation

githublaohu
Copy link
Contributor

No description provided.

}
DefaultMQProducer *defaultMQProducer = (DefaultMQProducer *) producer;
MQMessage *message = (MQMessage *) msg;
CSendCallback* cSendCallback = new CSendCallback(cSendSuccessCallback , cSendExceptionCallback);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cSendCallback should be deleted after send

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See MQClientAPIImpl::sendMessageAsync and AsyncCallbackWrap::operationComplete(90 line),and AsyncCallbackWrap::onException()[ 117 line]

typedef struct _CMQException_{
int error;
int line;
char file[MAX_MESSAGE_ID_LENGTH];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why use the same macro MAX_MESSAGE_ID_LENGTH to different member here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already processed


//typedef struct _CProducer_ _CProducer;
typedef struct CProducer CProducer;
typedef int(*QueueSelectorCallback)(int size, CMessage *msg, void *arg);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why need the parameter size, what's it function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QueueSelectorCallback is not asyn send callback function


virtual const char* GetType() const throw() { return m_type.c_str(); }

int GetLine() const throw(){ return m_line;}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there will no exception for GetLine and GetFile, so need delete throw() for them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already processed

exception.error = e.GetError();
exception.line = e.GetLine();
strncpy(exception.msg, e.what(), MAX_MESSAGE_ID_LENGTH - 1);
strncpy(exception.file, e.GetFile(), MAX_MESSAGE_ID_LENGTH - 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong alignment, format them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already processed

cout << "send Exception\n";
}
public:
virtual void onException(MQException& e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please format the alignment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already processed

}
virtual ~CSendCallback(){}
virtual void onSuccess(SendResult& sendResult) {
cout << "send onSuccess\n";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove stdout.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@githublaohu Could you please check this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks , Already processed


#include <string.h>
#include "RocketMQClient.h"
#include "CCommon.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why include C type head file here? it is better to remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already processed

try {
defaultMQProducer->send(*message ,cSendCallback);
} catch (exception &e) {
return PRODUCER_SEND_ONEWAY_FAILED;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error code is not right.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@githublaohu it is better to define a new error code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already processed

int line;
char file[MAX_EXEPTION_CHAR_LENGTH];
char msg[MAX_EXEPTION_CHAR_LENGTH];
char type[MAX_EXEPTION_CHAR_LENGTH];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is better to define the MACRO separately, and the length of msg maybe bigger than 256, IMO, maybe 512 is better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already processed

for (i = 0; i < 10; i++) {
printf("send one message : %d\n", i);
memset(DestMsg, 0, sizeof(DestMsg));
snprintf(DestMsg, 255, "New message body: index %d", i);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better to use sizeof(DestMsg) instead of 255.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DestMsg chang to body? variable need keep the same style, that is first word for variable with lowercase

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already processed

@ShannonDing ShannonDing merged commit e628502 into apache:master Jan 16, 2019
@ShannonDing ShannonDing added this to the 1.2.1 milestone Jan 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants