2

I am trying to improve the constructor of a serial port class. At the moment there are many overloads to handle different scenarios (port, baudrate, data bits, parity, open on creation, callbacks etc.). To allow the user of this class to pass only the parameters he/she needs in an arbitrary order, I started as follows:

#include <iostream> #include <string> #include <tuple> template <typename T, typename Tuple> struct has_type; template <typename T, typename... Us> struct has_type<T, std::tuple<Us...>> : std::disjunction<std::is_same<T, Us>...> {}; template <typename ...UnorderedArgs> std::string getPort(std::tuple<UnorderedArgs...> arg_set) { if constexpr (has_type<std::string, std::tuple<UnorderedArgs...>>::value) return std::get<std::string &&>(std::move(arg_set)); else return "NotSet"; } class SerialPort { public: template <typename... Args> SerialPort(Args ...args) : SerialPort(std::forward_as_tuple(std::move(args)...)) {} template <typename ...UnorderedArgs> SerialPort(std::tuple<UnorderedArgs...> arg_set) : SerialPort(getPort(std::move(arg_set))) {} SerialPort(const std::string &port) // [X] { std::cout << "SerialPort " << port << std::endl; } }; int main() { std::string port = "/dev/tty"; SerialPort sp(1, port); // without 1 the compiler would use [X] return 0; } 

This code sets the port to NotSet, so the part with if constexpr is not working as intended. How can this be fixed?

5
  • Hint: what's the type of arg_set? Commented Oct 24, 2019 at 12:30
  • 1
    I call this kind of code an "rvalue reference overdose". Basically a total oversaturation of rvalue references, for no useful reason whatsoever, and no apparent purpose. Even after straightening out the asked-about problem, it appears that the shown code will end up using an rvalue reference to a destroyed object (the function parameter). Hillarity ensues. As far as I can tell, every usage of std::move here is wrong. Commented Oct 24, 2019 at 12:34
  • Yeah, I think the move in SerialPort(std::forward_as_tuple(std::move(args)...)) kills it, as it turns std::string into std::string&& and std::is_same<std::string, std::string&&>::value == false Commented Oct 24, 2019 at 12:35
  • c-how-to-generate-all-the-permutations-of-function-overloads might interest you, or other alternative to simulate named parameter. Commented Oct 24, 2019 at 12:44
  • @SamVarshavchik It is doing weird things, but I don't actually see a dead reference issue. For example the getPort return (if it were actually used) move-constructs a string. It's somewhere else? Commented Oct 24, 2019 at 13:22

2 Answers 2

2

This code sets the port to NotSet, so the part with if constexpr is not working as intended. How can this be fixed?

It's a problem if references: when you check the presence of std::string in has_type, std::string is present but with a reference (&& given by std::move(), if I'm not wrong).

Solution: remove references

template <typename T, typename... Us> struct has_type<T, std::tuple<Us...>> : std::disjunction<std::is_same<T, std::remove_reference_t<Us>>...> {}; // ..................................^^^^^^^^^^^^^^^^^^^^^^^^^^^ 

-- EDIT --

As pointed by Jarod42 (thanks!), this way the name has_type isn't correct anymore.

And maybe you need an has_type that detect the presence of a type checking also the references...

Maybe is better maintain the original has_type and use std::remove_reference_t invoking it

// ............................................VVVVVVVVVVVVVVVVVVVVVVV if constexpr (has_type<std::string, std::tuple<std::remove_reference_t<UnorderedArgs>...>>::value) return std::get<std::string &&>(std::move(arg_set)); else return "NotSet"; 
Sign up to request clarification or add additional context in comments.

2 Comments

Whereas it might fix final code, it makes has_type to lie...
@Jarod42 - Good point. Yes, maybe is better remove references in the invocation.
1

Why are you restricting yourself to what C++ offers syntactically? This problem calls for variation on builder pattern and fluent API.

Use something like this:

class serial_port_options { /* private fields to hold options */ public: serial_port_options& set_port( int ); serial_port_options& set_baudrate( int ); serial_port_options& set_data_bits( int ); serial_port_options& set_parity( bool ); serial_port_options& set_open_on_creation( bool ); serial_port_options& set_callbacks( CALLBACK ); int get_port() const; int get_baudrate() const; int get_data_bits() const; bool get_parity() const; bool get_open_on_creation() const; CALLBACK get_callbacks() const; }; class SerialPort { public: SerialPort(const serial_port_options&); /* other logic and public interface */ }; 

Using the above, you can build your options object:

serial_port_options options; options.set_port(80).set_baudrate(65536).set_data_bits(7) .set_parity(false).set_open_on_creation(true); SerialPort port(options); 

1 Comment

Ah, I didn't think of that, but it's indeed the best solution to my real problem! However, I have accepted max66's answer since it solves the specific problem from the question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.