Fix and rename Config::find to Config::filter#452
Conversation
| Config Config::filter(const string &pro) | ||
| { | ||
| Config items; | ||
| Config items{true}; |
There was a problem hiding this comment.
I'm not really satisfied with the default argument approach. An alternative would be to leave the constructor unchanged and just call a clear on items before entering into the below for loop to start off from scratch.
| emplace_back(ConfigItem{"FB:", nullptr, nullptr, NXP_VID, 0x0152}); | ||
| emplace_back(ConfigItem{"FB:", nullptr, nullptr, 0x0483, 0x0afb}); | ||
| if (!construct_empty) { | ||
| emplace_back(ConfigItem{"SDPS:", "MX8QXP", nullptr, NXP_VID, 0x012F, 0x0002}); |
There was a problem hiding this comment.
Sorry, missed this pull request. Suggest
if (construct_empty)
return;
....
to avoid move whole emplace_back.
There was a problem hiding this comment.
No problem, there's always so much todo.
You suggestion is better. I changed the code accordingly.
| Config Config::find(const string &pro) | ||
| Config Config::filter(const string &pro) | ||
| { | ||
| Config items; |
There was a problem hiding this comment.
Thank you for your patch. Look like Config::find(const string &pro) is never used. we can remove it.
nxpfrankli
left a comment
nxpfrankli
left a comment
There was a problem hiding this comment.
Thank you for your patch. Look like Config::find(const string &pro) is never used. we can remove it.
Signed-off-by: markuspg <markuspg@users.noreply.github.com>
2 participants
I always scratched my head looking at
Config Config::find(const string &pro), wondering about its purpose. If I don't misinterpret it it creates a new instance ofConfig- holding all defaultConfigItems and then adds the ones of the current instance additionally which fit the passed protocol.Since I guess it should rather filter I modified the
Configconstructor to allow creating an instance without anyConfigItems and then adding just selected ones within thefilterfunction.