6
\$\begingroup\$

I would like to make the initialization code for the C++ std::map instance (table in the example below) a little more clean. Please lend me your wisdom.

Please use C++17.

Detail

Using a std::stringstream that can conveniently handle an enum called magic_enum, a static instance of std::map (table in the example below) is initialized in a dirty manner from a global function (trash::initialize() in the example below). I would like to write it using a constructor and an initialization list, but it is somehow difficult, so please tell me how to write it.

Source code

#include <iostream>
#include <map>
#include <sstream>
#include <iomanip>
#include "magic_enum.hpp" // https://github.com/Neargye/magic_enum/releases/download/v0.8.1/magic_enum.hpp
using namespace std;
enum TCPSTATUS {
 UNKNOWN, ESTABLISHED, SYN_SENT, SYN_RECV, FIN_WAIT1, FIN_WAIT2, TIME_WAIT, CLOSE, CLOSE_WAIT, LAST_ACK, LISTEN, CLOSING,
};
map<string, TCPSTATUS> table;
namespace trash {
 void* initialize() {
 magic_enum::enum_for_each<TCPSTATUS>([&] (auto val) {
 constexpr TCPSTATUS status = val;
 stringstream ss;
 ss << hex << uppercase << setw(2) << setfill('0') << magic_enum::enum_integer(status);
 table[ss.str()] = status;
 });
 return nullptr;
 }
 void* dummy = initialize();
}
int main(int argc, char* argv[]) {
 for (const auto& p: table) {
 cout << p.first << "->" << magic_enum::enum_name(p.second) << endl;
 }
 return 0;
}

Output

00->UNKNOWN
01->ESTABLISHED
02->SYN_SENT
03->SYN_RECV
04->FIN_WAIT1
05->FIN_WAIT2
06->TIME_WAIT
07->CLOSE
08->CLOSE_WAIT
09->LAST_ACK
0A->LISTEN
0B->CLOSING
G. Sliepen
68.6k3 gold badges74 silver badges179 bronze badges
asked Jan 6 at 7:21
\$\endgroup\$
1
  • \$\begingroup\$ The next time you want to restore to a previous version of the question, just use the Rollback link on the version you want to restore to. \$\endgroup\$ Commented Jan 6 at 14:55

3 Answers 3

7
\$\begingroup\$

Don't using namespace std.

using namespace std pollutes global namespace and leads to possible conflicts with other functions. Why is "using namespace std" considered bad practice?

You can't (yet) programmatically write an initialiser list, so use a range pipeline.

A std::initialiser_list can only be initialised from a braced-init-list (or copied from another std::initialiser_list), you can't turn some other container into one.

You can use ranges::to<std::map> to initialise a map from a range of pairs, and you can use ranges::views::transform to make your pairs from a range of enums, which magic_enum can provide. These were added to the standard in C++20 and C++23 respectively.

There's much better ways to format

fmt::format is a type-safe replacement for printf, that was accepted into C++20, fixing one of the major warts of C++ formatting. You don't need to use a stringstream.

Which ends up like this:

std::map<std::string, TCPSTATUS> table = magic_enum::enum_values<TCPSTATUS>() 
 | ranges::view::transform([](TCPSTATUS status){ return std::make_pair(fmt::format("{:02X}", magic_enum::enum_integer(status)), status); }) 
 | ranges::to<std::map>();
answered Jan 6 at 17:11
\$\endgroup\$
3
  • \$\begingroup\$ It's a very cool code, and it's close to ideal. But it's C++23. \$\endgroup\$ Commented Jan 6 at 23:25
  • \$\begingroup\$ @dameo The reference implementation is available in c++17: ericniebler.github.io/range-v3 similarly for format: github.com/fmtlib/fmt \$\endgroup\$ Commented Jan 7 at 9:16
  • \$\begingroup\$ Sorry. I didn't notice that you introduced a library that works with c++17. I have now confirmed that it works, so I accept this answer. \$\endgroup\$ Commented Jan 7 at 11:18
6
\$\begingroup\$

Try to avoid using namespace std;

using namespace std pollutes global namespace and leads to possible conflicts with other functions. Why is "using namespace std" considered bad practice?

About your question

I would like to write it using a constructor and an initialization list

Why don't you just write a function:

constexpr static auto construct_table()
{
 std::map<std::string, TCPSTATUS> output;
 magic_enum::enum_for_each<TCPSTATUS>(
 [&] (TCPSTATUS status)
 {
 std::stringstream ss;
 ss << std::hex << std::uppercase << std::setw(2) << std::setfill('0') << magic_enum::enum_integer(status);
 output[ss.str()] = status;
 }
 );
 return output;
}
answered Jan 6 at 12:38
\$\endgroup\$
9
  • \$\begingroup\$ Sorry, but that's just intentional. This kind of code needs to be short to make it difficult to read. If you are interested, I will correct the question code since it is troublesome. \$\endgroup\$ Commented Jan 6 at 12:48
  • 1
    \$\begingroup\$ @dameo please don't modify the code you posted. \$\endgroup\$ Commented Jan 6 at 13:09
  • 2
    \$\begingroup\$ I restored the code. \$\endgroup\$ Commented Jan 6 at 13:18
  • \$\begingroup\$ RVO has been guaranteed since C++17 and is a better choice, but this method depends on it and I didn't want to actively use it. \$\endgroup\$ Commented Jan 6 at 15:22
  • 2
    \$\begingroup\$ @daemon "RVO has been guaranteed since C++17 and is a better choice, but this method depends on it and I didn't want to actively use it." Given the first part of that sentence, I don't understand why you did not want to use it. Also, note that even if RVO wasn't a thing, it still would be fine to initialize a std::map from a temporary returned by a function, as it would just do a cheap move. \$\endgroup\$ Commented Jan 6 at 19:38
3
\$\begingroup\$

How about something like this using iterator?

Explanation

It seems impossible to write using a constructor and an initialization list even in C++23, so I searched for a similar method and ended up creating my own iterator. When I created my own iterator, the code became longer than I expected, but I was able to cleanly initialize std::map with a single constructor.

#include <iostream>
#include <map>
#include <sstream>
#include <iomanip>
#include <iterator>
#include <utility>
#include <type_traits>
#include "magic_enum.hpp" // https://github.com/Neargye/magic_enum/releases/download/v0.8.1/magic_enum.hpp
enum TCPSTATUS {
 UNKNOWN, ESTABLISHED, SYN_SENT, SYN_RECV, FIN_WAIT1, FIN_WAIT2, TIME_WAIT, CLOSE, CLOSE_WAIT, LAST_ACK, LISTEN, CLOSING,
};
using tcpstatus_values_iterator = decltype(magic_enum::enum_values<TCPSTATUS>().begin()); 
class pair_generator_iterator {
 tcpstatus_values_iterator _iter;
 static std::string generate_key(TCPSTATUS i) {
 std::stringstream ss;
 ss << std::hex << std::uppercase << std::setw(2) << std::setfill('0') << magic_enum::enum_integer(i);
 return ss.str();
 }
public:
 using original_value_type = TCPSTATUS;
 using key_type = decltype(generate_key(UNKNOWN));
 using iterator_category = std::input_iterator_tag;
 using difference_type = int;
 using value_type = std::pair<key_type, original_value_type>;
 using pointer = value_type*;
 using reference = value_type&;
 pair_generator_iterator(tcpstatus_values_iterator iter): _iter{iter} {}
 value_type operator*() const {
 original_value_type v = *_iter;
 return make_pair(generate_key(v), v);
 }
 pair_generator_iterator& operator++() {
 _iter++;
 return *this;
 } 
 friend bool operator== (const pair_generator_iterator& a, const pair_generator_iterator& b) { return a._iter == b._iter; }
 friend bool operator!= (const pair_generator_iterator& a, const pair_generator_iterator& b) { return !(a == b); }
};
std::map<std::string, TCPSTATUS> table{
 pair_generator_iterator(magic_enum::enum_values<TCPSTATUS>().begin()),
 pair_generator_iterator(magic_enum::enum_values<TCPSTATUS>().end())
};
int main(int argc, char* argv[]) {
 for (const auto& p: table) {
 std::cout << p.first << "->" << magic_enum::enum_name(p.second) << std::endl;
 }
 return 0;
}
answered Jan 7 at 2:36
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.