Skip to content
This repository was archived by the owner on Jul 8, 2022. It is now read-only.

Conversation

@bourtemb
Copy link
Member

No description provided.

@bourtemb
Copy link
Member Author

This is the equivalent of the following commit from tango-9-lts branch for the master branch:
e529990

Copy link
Member

@Ingvord Ingvord left a comment

Choose a reason for hiding this comment

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

Looks like it is an utility method that is useful for any C++ type enum. I would put it into a dedicated header file as a template utility function, smth like

template<typename Enum, typename EnumNames>
Enum enum_value_of(const std::string& name){
//TODO ...
}

event = (*argin)[3];

//
// Check event type validity
Copy link
Member

Choose a reason for hiding this comment

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

Whenever there is a comment in the code describing what it is doing - extract a method.

size_t nb_event_type = sizeof(EventName)/sizeof(char *);
bool found = false;

for (size_t loop = 0;loop < nb_event_type;loop++)
Copy link
Member

Choose a reason for hiding this comment

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

loop should be renamed to i

}
}

if (found == false)
Copy link
Member

Choose a reason for hiding this comment

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

Should be if(!found)

stringstream ss;
ss << "The event type you sent (" << event << ") is not valid event type";

Except::throw_exception(API_WrongNumberOfArgs,ss.str(),"DServer::zmq_event_subscription_change");
Copy link
Member

Choose a reason for hiding this comment

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

Can be simplified to

Except::throw_exception(
    API_WrongNumberOfArgs,
    "The event type you sent (" + event +  ") is not  valid event type", //TODO name valid types
    "DServer::zmq_event_subscription_change");

@bourtemb
Copy link
Member Author

I'm sorry, I don't understand the following comment:

Looks like it is an utility method that is useful for any C++ type enum. I would put it into a dedicated header file as a template utility function, smth like

template<typename Enum, typename EnumNames>
Enum enum_value_of(const std::string& name){
//TODO ...
}

Can you be more explicit, please? To what part of the code are you referring to for this comment?

@Ingvord
Copy link
Member

Ingvord commented Dec 15, 2016 via email

Copy link
Member

@Ingvord Ingvord left a comment

Choose a reason for hiding this comment

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

Looks OK to me!

string check_event = event;
transform(check_event.begin(),check_event.end(),check_event.begin(),::tolower);

string::size_type pos_check = check_event.find(EVENT_COMPAT);
Copy link
Member

Choose a reason for hiding this comment

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

string::size_type can be replaced with auto

Ingvord pushed a commit that referenced this pull request Dec 15, 2016
@Ingvord Ingvord merged commit cea3f7b into tango-controls:master Dec 15, 2016
Ingvord pushed a commit that referenced this pull request Dec 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants