# Tag Info

17

template <typename Less, typename T, typename... Ts> constexpr const T& min(Less less, const T& a, const T& b, const Ts&... rems) { This function requires a minimum of 2 elements. A minimum element will exist if the user provides a single argument variadic list. Consider handling that. auto& min1 = min(std::less<>{}, 4, 5); ...

12

Below is how I would clean this up, or maybe partially re-write [live example]: // helpers template <typename T> struct id { using type = T; }; template <typename T> using type_of = typename T::type; template <size_t... N> struct sizes : id <sizes <N...> > { }; // choose N-th element in list <T...> template <size_t ...

12

This looks nice! Two issues I see here, When compiling your template with clang, it refuses the if constexpr dispatch for every recursive instantiation with sizeof...(rems) > 1, e.g. error: constexpr if condition evaluates to 2, which cannot be narrowed to type bool [-Wc++11-narrowing] gcc seems to accept this, but the fix is quite simple, just be ...

10

Bugs There is no call to va_end() The va_start() macro must be called first, and it initializes ap, which can be passed to va_arg() for each argument to be processed. Calling va_end() signals that there are no further arguments, and causes ap to be invalidated. Note **that each call to va_start() must be matched by a call to va_end(), from within the ...

10

Yes, it is a good example Variadic templates are associated with recursion by similarity to functional programming, where an recursion is a simple method of operating on sequences. Direct expansion is akin to a map or fold, or some other higher-order function. It can be simplified even more If you have a similar function /* find value or return default *...

9

It works well, according to my simple test program: #include <functional> int main() { return min(std::less<int>(), 2, 0, 3); } I suggest that when you present code for review, you include the tests, too - I imagine that your testing is much more thorough than mine, and I'm too lazy to re-create the full suite. It also helps reviewers ...

8

In C++14, it's possible to implement a tuple in a very concise way. However, you lose some functionality of std::tuple (constexprness and some perfect forwarding) because of some limitations of lambdas. The basic idea is to use a lambda capture as a fast compiler-generated struct. Here's a naive implementation just to give you the idea, but you should look ...

8

Going from case 'd' to case 'c', you have these lines: 'd': PRINTCHAR(acTemp[i]); nChars++; 's': PRINTCHAR(*pcTemp); nChars++; 'c': PRINTCHAR(cTemp); nChars++; Why did you all of a sudden change the pattern? In my opinion, you should be using the pattern that you used in option 'c' because it is clean and spread out, rather than jammed into one line. ...

8

Don't invent your own language through macros GLOBAL and LOCAL might look cute but they make the code harder to read for everyone else. Not to mention the name-space pollution caused by such macros. You say that they both expand to static. That seems wrong. A function declared static is not “very global” – it is private to its translation unit. A local ...

8

Issues (for me) I get a couple of errors when I try and compile. This error I have a feeling is caused because you are using a more up to date compiler (I presume this is a C++17 feature). if constexpr (sizeof...(Tail) > 0) { ^^^^^^^^^ This problem causes my second error (because I remove constexpr to make it compile): rt.cpp:12:6: note: candidate ...

8

General design Currently, your function is defined to have "undefined behavior" if the number of arguments is wrong. This is sub-optimal. Checking is trivial in this case, so report the problem in some way instead of producing strange output. I could make it a run-time error, but that would add run-time cost and thus violate C++'s philosophy of ...

7

Termination expression I have a pet peeve when people use functions/expressions in their termination clause which have constant value: for(int i = 0; i < strlen(acTemp); i++) Here you are going to call strlen N times, making this loop an $O(n^2)$ loop for no reason. Much better would be: int len = strlen(acTemp); for(int i = ...

7

Since you're implementing a traits-like check, I would typically expect your traits class types to be derived from std::true_type and std::false_type. This is certainly not required, and overall this implementation doesn't bug me. I was slightly surprised to see std::tuple<> called out explicitly, but that doesn't seem to change the overall correctness ...

7

Your current strategy will cause more reallocations than you need to, which is not efficient. You already have all of your vector parameters, so you simply need to get their size, and reserve that amount for the output vector. template<typename V1, typename V2, typename... Rest> std::size_t get_reserve_amount( V1 const& v1, V2 const& v2, Rest ...

7

Is there a situation where the compiler will not optimize away the wrapper because the wrapper and the original function have different semantics? Yes to the first part and no to the second. Any optimizing compiler will inline your wrapper, as it consists of only one return statement. But on the other hand, the original function might have different ...

6

struct has public inheritance By default, struct has public inheritance. Therefore, you don't need to manually specify it every time. You can write shorter (and so more readable) code: template <> struct template_all<> : std::true_type {}; template <typename ... Types> struct template_all<std::false_type, Types...> : std::...

6

Always return allocated string or never do it This code here: if (count < 1) { return ""; } is bad because you return a static string whereas the normal case returns a string allocated by malloc. If the caller then tries to free this static string, it will cause some kind of error. You can fix this by either returning an allocated empty string: ...

6

Comment First question is why I would use this instead of the standard operator<< that we are all very familiar with? Code Review Reserved Identifiers Identifiers with a prefix underscore and an initial capital letter are reserved in all contexts for the implementations. i.e. You are not allowed to use them. So _INPUT_VALIDATION_HPP is an illegal ...

6

About the seed function, though, I must warn you that you might be surprised at some point by which overload the compiler will choose. Unless you really know what you're doing, having several template overloads, one of which exposes a default type, looks dangerous. If you multiply the overloads, the if constexpr c++17-feature is a better way to go, because ...

6

I think this function is unnecessary. We can deal with defective classes (that don't properly implement the standard LessThanComparable concept) by either fixing them (preferable) or by providing a comparator argument to std::max: auto const b_lessthan = [](const B& a, const B& b){ return !(a>=b); }; std::max({B{}, B{}, B{}, B{}, B{}}, ...

5

The biggest thing that sticks out at me is that you're passing all of your Args... parameters by value, thus pretty much making std::forward useless here. To make use of std::forward, the reference type needs to be deduced from the calling context. By itself, std::forward really doesn't do anything except a static_cast to the deduced reference type. ...

5

You can avoid problems related to variadic macros and problems related to C-style variadic functions altogether by creating a function that takes an std::initializer_list directly: template<class T> std::vector<T> select(const std::vector<T>& vec, std::initializer_list<std::size_t> indices) { std::vector<T> ret; for ...

5

In no particular order: Normally, you shouldn't define your own copy and move constructors and assignment operators unless you need special processing in them. Which in your case you don't; the implicitly generated ones will do exactly what you need. So just remove them and with them, a potential source of bugs or inefficiencies. Unfortunately, this doesn'...

5

I guess you are aware of std::bitset and you have your own reasons for doing this. Your pack seems fine, but unpack requires the user to set up a particular number of bool variables, which is not convenient. An alternative could be to use an std::array for the output: template<size_t N> using size = std::integral_constant<size_t, N>; template&...

5

Missing overflow check One serious flaw that I see in this code is that the user needs to determine the number of parameters to unpack. This is a no go for a function whose output depends on a runtime input. Even worse there is no check that would tell the user that the actual number overflows the number of bits given to be unpacked into. One possible ...

5

One possibility would be to use an std::initializer_list: #include <initializer_list> #include <numeric> #include <iostream> double average(std::initializer_list<double> input) { double sum = std::accumulate(input.begin(), input.end(), 0.0); return sum / input.size(); } int main() { std::cout << "Average: " <...

5

Technicality Pedantically speaking, the GP_get_type function has undefined behavior if the pointer it gets isn't a pointer into (or one past the end of) GP_format (see §6.5.8/5 Relational operators). Simple enough to fix: wrap all the arguments, including the first one and any string literal with GP. You could enforce that with an assertion in GP_get_type ...

5

It appears code is simply exclusive or-ing the arguments. Sounds like a "add without carry". "Is my logic for the implementation correct?" Appears correct except the range of acceptable values is not clearly stated. Gotcha 1. What is UINT? If UINT is narrower than int/unsigned, the arguments will be widened and va_arg(valist,UINT); is problematic. ...

5

Automate consistency p = malloc(sizeof(char*) * count); len_array = calloc(count, sizeof(size_t)); If you instead say p = malloc(count * sizeof *p); len_array = calloc(count, sizeof *len_array); You can change the types of p and len_array without having to change these calls as well. You should also check that these functions are ...

5

A few points of criticism of your approach: Prefer protected to private This isn't really about your overall design, but - unless you have very good reason, use protected rather than private member. In few cases is it useful to hide data from your subclasses. "Where did my data go?!" Suppose you have vectors types T_1 and T_2 in your tuple, and you add() ...

Only top voted, non community-wiki answers of a minimum length are eligible