Skip to content

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Jul 26, 2019

Using a direct list initialization for struct Arg objects makes the constructor needless.

This PR has been split out from #16097 (see: #16097 (comment)).

@hebasto
Copy link
Member Author

hebasto commented Jul 26, 2019

ping @ryanofsky @promag

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

ACK.

LOCK(cs_args);
std::map<std::string, Arg>& arg_map = m_available_args[cat];
auto ret = arg_map.emplace(name.substr(0, eq_index), Arg(name.substr(eq_index, name.size() - eq_index), help, debug_only));
auto ret = arg_map.emplace(name.substr(0, eq_index), Arg{name.substr(eq_index, name.size() - eq_index), help, debug_only});
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't test at the moment but I think you could drop Arg?

Copy link
Member Author

@hebasto hebasto Jul 26, 2019

Choose a reason for hiding this comment

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

Can't test at the moment but I think you could drop Arg?

It won't work if a type is automatically deduced: auto ret = ...

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to keep it. Three ascii chars are not a bloat

{
std::string m_help_param;
std::string m_help_text;
bool m_debug_only;
Copy link
Member

Choose a reason for hiding this comment

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

Can those be const to enforce setting them in the constructor?

LOCK(cs_args);
std::map<std::string, Arg>& arg_map = m_available_args[cat];
auto ret = arg_map.emplace(name.substr(0, eq_index), Arg(name.substr(eq_index, name.size() - eq_index), help, debug_only));
auto ret = arg_map.emplace(name.substr(0, eq_index), Arg{name.substr(eq_index, name.size() - eq_index), help, debug_only});
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to keep it. Three ascii chars are not a bloat

@hebasto hebasto closed this Jul 27, 2019
@hebasto hebasto deleted the 20190726-remove-arg-ctor branch August 2, 2019 17:26
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants